Re: [PATCH] shell32: do not crash wine control.exe nonexisting
Hi Marcus, On 06/06/12 10:17, Marcus Meissner wrote: Hi, wine control.exe joy crashed in this line, as it should be wine control.exe joy.cpl. The list is just not setup yet as we fail... Ciao, Marcus --- dlls/shell32/control.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/dlls/shell32/control.c b/dlls/shell32/control.c index cb080d5..b3eddb6 100644 --- a/dlls/shell32/control.c +++ b/dlls/shell32/control.c @@ -55,7 +55,7 @@ void Control_UnloadApplet(CPlApplet* applet) } if (applet-proc) applet-proc(applet-hWnd, CPL_EXIT, 0L, 0L); FreeLibrary(applet-hModule); -list_remove( applet-entry ); +if (applet-entry.next) list_remove( applet-entry ); HeapFree(GetProcessHeap(), 0, applet-cmd); HeapFree(GetProcessHeap(), 0, applet); } The way Wine standard list implementation works, it should not be NULL here. It seems that list_init call is missing where the CPlApplet is allocated. Cheers, Jacek
Re: [PATCH] shell32: do not crash wine control.exe nonexisting
On Wed, Jun 06, 2012 at 11:31:52AM +0200, Jacek Caban wrote: Hi Marcus, On 06/06/12 10:17, Marcus Meissner wrote: Hi, wine control.exe joy crashed in this line, as it should be wine control.exe joy.cpl. The list is just not setup yet as we fail... Ciao, Marcus --- dlls/shell32/control.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/dlls/shell32/control.c b/dlls/shell32/control.c index cb080d5..b3eddb6 100644 --- a/dlls/shell32/control.c +++ b/dlls/shell32/control.c @@ -55,7 +55,7 @@ void Control_UnloadApplet(CPlApplet* applet) } if (applet-proc) applet-proc(applet-hWnd, CPL_EXIT, 0L, 0L); FreeLibrary(applet-hModule); -list_remove( applet-entry ); +if (applet-entry.next) list_remove( applet-entry ); HeapFree(GetProcessHeap(), 0, applet-cmd); HeapFree(GetProcessHeap(), 0, applet); } The way Wine standard list implementation works, it should not be NULL here. It seems that list_init call is missing where the CPlApplet is allocated. The error handling that calls this leaves the Control_LoadApplet function before it is attached to the list head. list_add_head( panel-applets, applet-entry ); is not called before leaving the function. No other list_ functions are used by Control_LoadApplet. Not sure if this is right, or if there should be a entry init? Ciao, Marcus
Re: [PATCH] shell32: do not crash wine control.exe nonexisting
On 06/06/12 11:34, Marcus Meissner wrote: On Wed, Jun 06, 2012 at 11:31:52AM +0200, Jacek Caban wrote: Hi Marcus, On 06/06/12 10:17, Marcus Meissner wrote: Hi, wine control.exe joy crashed in this line, as it should be wine control.exe joy.cpl. The list is just not setup yet as we fail... Ciao, Marcus --- dlls/shell32/control.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/dlls/shell32/control.c b/dlls/shell32/control.c index cb080d5..b3eddb6 100644 --- a/dlls/shell32/control.c +++ b/dlls/shell32/control.c @@ -55,7 +55,7 @@ void Control_UnloadApplet(CPlApplet* applet) } if (applet-proc) applet-proc(applet-hWnd, CPL_EXIT, 0L, 0L); FreeLibrary(applet-hModule); -list_remove( applet-entry ); +if (applet-entry.next) list_remove( applet-entry ); HeapFree(GetProcessHeap(), 0, applet-cmd); HeapFree(GetProcessHeap(), 0, applet); } The way Wine standard list implementation works, it should not be NULL here. It seems that list_init call is missing where the CPlApplet is allocated. The error handling that calls this leaves the Control_LoadApplet function before it is attached to the list head. list_add_head( panel-applets, applet-entry ); is not called before leaving the function. No other list_ functions are used by Control_LoadApplet. Not sure if this is right, or if there should be a entry init? This usage of list is broken here as well. list_init should be called before list_add_head (so calling it early in initialization code will fix both problems). Cheers, Jacek
Re: [PATCH] shell32: do not crash wine control.exe nonexisting
Jacek Caban ja...@codeweavers.com writes: This usage of list is broken here as well. list_init should be called before list_add_head (so calling it early in initialization code will fix both problems). There's no reason to call list_init on list entries, it should only be called on the actual list. -- Alexandre Julliard julli...@winehq.org
Re: [PATCH] shell32: do not crash wine control.exe nonexisting
On 06/06/12 11:52, Alexandre Julliard wrote: Jacek Caban ja...@codeweavers.com writes: This usage of list is broken here as well. list_init should be called before list_add_head (so calling it early in initialization code will fix both problems). There's no reason to call list_init on list entries, it should only be called on the actual list. Oh, right, sorry for misleading. I made wrong assumptions looking at the patch. Jacek
Re: [PATCH] shell32: do not crash wine control.exe nonexisting
On Wed, Jun 06, 2012 at 11:56:06AM +0200, Jacek Caban wrote: On 06/06/12 11:52, Alexandre Julliard wrote: Jacek Caban ja...@codeweavers.com writes: This usage of list is broken here as well. list_init should be called before list_add_head (so calling it early in initialization code will fix both problems). There's no reason to call list_init on list entries, it should only be called on the actual list. Oh, right, sorry for misleading. I made wrong assumptions looking at the patch. So, Alexandre, is my patch right or wrong? And if wrong, can you quickly fix it:) Ciao, Marcus
Re: [PATCH] shell32: do not crash wine control.exe nonexisting
Marcus Meissner meiss...@suse.de writes: On Wed, Jun 06, 2012 at 11:56:06AM +0200, Jacek Caban wrote: On 06/06/12 11:52, Alexandre Julliard wrote: Jacek Caban ja...@codeweavers.com writes: This usage of list is broken here as well. list_init should be called before list_add_head (so calling it early in initialization code will fix both problems). There's no reason to call list_init on list entries, it should only be called on the actual list. Oh, right, sorry for misleading. I made wrong assumptions looking at the patch. So, Alexandre, is my patch right or wrong? And if wrong, can you quickly fix it:) The patch would work, but it's not very clean. It's better to avoid destroying objects that are not yet on the list. I have some more cleanups for that code that should address the issue. -- Alexandre Julliard julli...@winehq.org
