Previous Thread
Next Thread
Print Thread
Page 144 of 529 1 2 142 143 144 145 146 528 529
Re: SVN builds - new driver flood [Re: Lord Nightmare] #61390 04/29/10 07:04 PM
Joined: Mar 2001
Posts: 16,387
R
R. Belmont Offline
Very Senior Member
Offline
Very Senior Member
R
Joined: Mar 2001
Posts: 16,387
How can the SDL OSD code prevent him from fixing the Windows OSD? That's ridiculous. You can't even build SDLMESS with newui.

Re: SVN builds - new driver flood [Re: R. Belmont] #61391 04/29/10 07:18 PM
Joined: Mar 2006
Posts: 1,035
L
Lord Nightmare Offline
Very Senior Member
Offline
Very Senior Member
L
Joined: Mar 2006
Posts: 1,035
It has to do with editing/fixing dependencies, not actually deeply touching the code. I think.
There's certain bits of newui/wimgtool code which need to be able to use osd_free but can't because of the way the dependencies work iirc.
LN


"When life gives you zombies... *CHA-CHIK!* ...you make zombie-ade!"
Re: SVN builds - new driver flood [Re: Lord Nightmare] #61392 04/29/10 07:27 PM
Joined: Mar 2001
Posts: 16,387
R
R. Belmont Offline
Very Senior Member
Offline
Very Senior Member
R
Joined: Mar 2001
Posts: 16,387
Well, it's not like I'm hard to contact either here or on MW.

Re: SVN builds - new driver flood [Re: R. Belmont] #61393 04/29/10 08:21 PM
Joined: Sep 2001
Posts: 534
F
Firewave Offline
Senior Member
Offline
Senior Member
F
Joined: Sep 2001
Posts: 534
I sent the complete patch (including the SDL parts) to MAMEdev and also posted about the issue on the MESSdev mailing list before I started working on it to get feedback on that issue. I also described what I did and why I did it. The patch didn't make u3, but so did nothing I sent to MAMEdev after u2. So I have no idea, if the changes were rejected or else. From people, that are on the list I heard, that there was concern about some of my changes and there's a backlog post-u2 changes and there's work being done to get those in, but also requests to submit it again. Since I am not sure what actually made it and lack of time I decided for u4.

To clear up the comment by Lord Nightmare. The patch was neither incomplete nor partial, but I might have overseen some memory leaks in the code (only possible to say with further code reviews or memory leak detection tools - static and/or run-time). The code is question is part of the OSD layer and exists in Windows and SDL, so I had to touch both. In both cases it's a change from one malloc variant to another (standard malloc() to osd_malloc()) and the fixing of the free calls (which were in most cases totally wrong and caused memory leaks all over the place - not just with newui/wimgtool, but the core as well).

The crash is newui happened because I accidentally committed code, that was from my patched tree, which needs osd_free() to be used. The fix would just have been to use global_free() instead (just for consistency since it does *nothing* to just malloc'd memory), but since it causes link problems with some tools there has to be an intentional memory leak for now. I will apply the patch later since I ran into a memory leak (in my non-fixed tree) when checking the fix.

Re: SVN builds - new driver flood [Re: Firewave] #61395 04/29/10 08:36 PM
Joined: Mar 2001
Posts: 16,387
R
R. Belmont Offline
Very Senior Member
Offline
Very Senior Member
R
Joined: Mar 2001
Posts: 16,387
The major problem with your patch is that it was submitted as a single giant piece and that makes it extremely difficult to review. If you could split it into a series of self-contained patches that each fix one leak or one module it could be applied much more efficiently.

Re: SVN builds - new driver flood [Re: R. Belmont] #61396 04/29/10 08:41 PM
Joined: Sep 2001
Posts: 534
F
Firewave Offline
Senior Member
Offline
Senior Member
F
Joined: Sep 2001
Posts: 534
Well, it all belongs together. I admit there's a couple of memory leak fix in there, that not directly apply to the strconv.c, but when changing the allocation function the deallocation function should be fixed as well in that commit and not separately.
If I split it up in three patches, the first patch might cause breakage, because it relies on the changes needed in the second part (the third part would be the stuff, that I found along the way).

Re: SVN builds - new driver flood [Re: Firewave] #61399 04/29/10 09:06 PM
Joined: Sep 2001
Posts: 534
F
Firewave Offline
Senior Member
Offline
Senior Member
F
Joined: Sep 2001
Posts: 534
I have just been told my patch was already applied to MAME SVN so I applied the core changes to MESS SVN as well and will add all the changes to the MESS/newui/wimgtool/winui code.
Problem solved...no problem at all actually.

Re: SVN builds - new driver flood [Re: Firewave] #61400 04/29/10 09:18 PM
Joined: Sep 2001
Posts: 534
F
Firewave Offline
Senior Member
Offline
Senior Member
F
Joined: Sep 2001
Posts: 534
I just applied all the changes. Please report any breakage. These changes have been working fine for me using VS2010 and GCC the past week. I didn't test SDL yet though...

Re: SVN builds - new driver flood [Re: Firewave] #61401 04/29/10 09:21 PM
Joined: Mar 2001
Posts: 16,387
R
R. Belmont Offline
Very Senior Member
Offline
Very Senior Member
R
Joined: Mar 2001
Posts: 16,387
SDL shouldn't work if the patch was applied to MAME SVN, as I explicitly NAKed the SDL portion until I could review it. I'll have to investigate.

Re: SVN builds - new driver flood [Re: R. Belmont] #61402 04/29/10 09:56 PM
Joined: Sep 2001
Posts: 534
F
Firewave Offline
Senior Member
Offline
Senior Member
F
Joined: Sep 2001
Posts: 534
I just compiled latest SVN with VS2010 and smoke tested MESS, newui, wimgtool and messui. Moving on to GCC compile.

Page 144 of 529 1 2 142 143 144 145 146 528 529

Who's Online Now
1 registered members (Dam0), 165 guests, and 1 spider.
Key: Admin, Global Mod, Mod
ShoutChat Box
Comment Guidelines: Do post respectful and insightful comments. Don't flame, hate, spam.
Forum Statistics
Forums9
Topics8,719
Posts114,611
Members4,873
Most Online510
Aug 26th, 2019
Powered by UBB.threads™ PHP Forum Software 7.7.3