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
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
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
Jacek Caban jacek@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.
On 06/06/12 11:52, Alexandre Julliard wrote:
Jacek Caban jacek@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
On Wed, Jun 06, 2012 at 11:56:06AM +0200, Jacek Caban wrote:
On 06/06/12 11:52, Alexandre Julliard wrote:
Jacek Caban jacek@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
Marcus Meissner meissner@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 jacek@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.