Prashant-Jonny / win7shell

Automatically exported from code.google.com/p/win7shell
0 stars 0 forks source link

Winamp crash on exit #270

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The plugin crashes on winamp exit when running it under account with no 
administrative rights. It seems it's trying to dereference some invalid 
pointer. 

Windows 7, 64-bit

Original issue reported on code.google.com by ikomi...@gmail.com on 17 Jan 2010 at 7:29

Attachments:

GoogleCodeExporter commented 9 years ago
Same thing here, would've debugged it myself if it was humanly possible to 
compile
1.12. Even after tracking dependencies, creating a project with the proper 
include
paths, and a couple of edits, I still have tons of resource errors and misc. 
warnings
in VC++ 2008.

Original comment by danielk...@gmail.com on 10 Feb 2010 at 5:34

GoogleCodeExporter commented 9 years ago
Well I'll look into it... again :(
You can't compile it because you miss all the icons and other resources :/

Original comment by att...@gmail.com on 10 Feb 2010 at 5:41

GoogleCodeExporter commented 9 years ago
Well, after some Windows reinstallations I've concluded that this happens only 
after a 
clean install of Winamp 5.57. There is a workaround - install Winamp 5.56 first 
and 
then upgrade it to the last version. But if you have already installed 5.57 
downgrading  
it to 5.56 will not fix the problem. atti86, if you want to debug it you'll 
need a 
clean install of 5.57, not an upgraded one - 5.57 must be the very first Winamp 
version 
installed on the machine. I think the older versions change the user settings 
in some 
way which allows the plugin to work normally with 5.57, but this is just an 
assumption 
since I also cannot debug it...

Original comment by ikomi...@gmail.com on 10 Feb 2010 at 10:47

GoogleCodeExporter commented 9 years ago
I'll try next week when I have access to the code :)

Original comment by att...@gmail.com on 10 Feb 2010 at 10:55

GoogleCodeExporter commented 9 years ago
@atti86:

Any chance you can make the icons available? What's the point of releasing the 
source
if nobody can compile it?

Thanks anyway.

Original comment by danielk...@gmail.com on 11 Feb 2010 at 11:14

GoogleCodeExporter commented 9 years ago
Okay, if you are capable of debugging on your PC, it would be really nice... If 
not,
I'll do a clean install and see if I can track it down :)

Here is the project, I hope I set it up correctly so you can compile it. You 
still
need some library dependencies. Tell me if you need something else.

http://win7shell.googlecode.com/files/gen_win7shell.zip

Original comment by att...@gmail.com on 16 Feb 2010 at 9:42

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Think I've found it: gen_win7shell.cpp, getWinampINIPath():

Winamp seems not to return a proper INI path while it's closing. Due to lack of
validation, this aforementioned null pointer is dereferenced by
MultiByteToWideChar(), causing the fault.

Solution: check for a null pointer.

====
char *dir=(char*)SendMessage(wnd,WM_WA_IPC,0,IPC_GETINIDIRECTORY);

if(!dir) {
  wini.clear();
} else {

...
====

Original comment by danielk...@gmail.com on 17 Feb 2010 at 9:39

GoogleCodeExporter commented 9 years ago
Well I'm so sick of this bullshit ini path... there's no clear way of getting 
it, so
you have to dig it around ...

Meh, I've compiled it with your check, use this one if it helps. DrO said there 
will
be a unicode API for getting the path, I'll see if I can use that to fix this up
properly.

Attached only the plugin .dll.

Original comment by att...@gmail.com on 17 Feb 2010 at 10:33

Attachments:

GoogleCodeExporter commented 9 years ago
I can also confirm what danielkza2 has found out. Actually the problem is that 
plugin.hwndParent is a dangling handle because the Winamp window is already 
destroyed 
at this time and SendMessage() is called with an invalid handle. I've checked 
that 
with Spy++, GetLastError() also returns "Invalid window handle."(error code 
1400). 
Interestingly, MS says nothing about the return value of SendMessage() in such 
a case 
- "The return value specifies the result of the message processing; it depends 
on the 
message sent.".
In this case it returns 0 which is then successfully casted to (char*)NULL but 
if it 
returns another value (-1 for example) 'if (!dir)' will no longer work...

Original comment by ikomi...@gmail.com on 18 Feb 2010 at 2:01

GoogleCodeExporter commented 9 years ago
I already have a global string object that holds the path to the settings file, 
so
there is no need to re-generate it every time, we can just use that one... 

I added this to the start of the function:

    if (!W_INI.empty())
    {
        std::wstring::size_type pos = 0;
        pos = W_INI.find(L"\\Plugins\\");
        if (pos != std::wstring::npos)
        {
            return W_INI.substr(pos);
        }
    }

Hope it works out now :-s

Original comment by att...@gmail.com on 18 Feb 2010 at 10:48

Attachments:

GoogleCodeExporter commented 9 years ago
Hey, just tried that latest gen_win7shell.dll as attached on post #11 and there 
are
no more crashes on close.

Great work :)

Original comment by mafriki on 21 Feb 2010 at 12:18

GoogleCodeExporter commented 9 years ago
Thank these other guys who found the bug... I don't have it, because I have a
different Winamp path. I think I will release a new version after adding some 
other
minor modifications next week.

Marking this as Fixed.

Original comment by att...@gmail.com on 21 Feb 2010 at 12:24

GoogleCodeExporter commented 9 years ago
It works flawlessly now. Thanks a lot for the fix :)

Original comment by ikomi...@gmail.com on 22 Feb 2010 at 5:10