x11drv: XDND: Fix file drop to properly support file:/// URIs(debugging problem)

[Maciej Katafiasz]

On tor, 2007-03-22 at 12:02 +0800, Dmitry Timoshkov wrote:
> Please use the same indentation style as the existing does, i.e. 4 spaces,
> not 2.

Yep, I didn't fix that in that patch, I just forgot to set the indent
style properly, and it's not a patch ready for submission anyway. But
noted.

> > +    pathlen = MultiByteToWideChar(CP_UNIXCP, 0, filename, len, NULL, 0);
> > +
> > +    wfn = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, pathlen);
> 
> You allocate not enough space for a wide char string.

Æh, my bad, I blame it on the confusing WCHAR vs. bytes semantics. I
thought it was returned in bytes, now I see that particular one is
WCHARs.

> > +    TRACE("Convert to WCHAR: filename: %s, len: %d\n", debugstr_a(filename), pathlen);
> > +    pathlen = MultiByteToWideChar(CP_UNIXCP, 0, filename, len, wfn, pathlen);
> > +    if(GetLastError())
> > +    {
> > +      TRACE("Can't convert to WCHAR: %d\n", GetLastError());
> > +      goto clean_wfn;
> > +    }
> 
> This is not an appropriate way of testing for an API failure.

What is inappropriate and how should I fix it?

> > +    TRACE("WCHAR filename: %s\n", debugstr_w(wfn));
> > +    wpath = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, pathlen * sizeof(WCHAR));
> 
> Memory space calculations and allocations don't look right all over the place.

Apart from the one above, I couldn't find any mistakes. I tried looking
for WCHARs vs. bytes too.

> > -        strcpy(((char*)lpDrop)+lpDrop->pFiles, path);
> > +        memcpy(((char*)lpDrop)+lpDrop->pFiles, (char*)wpath, fullpathlen * sizeof(WCHAR));
> 
> Use lstrcpyW here instead of memcpy.

Is there any particular benefit of doing so, when I have the size known
beforehand?

Cheers,

-- 
There are only two things wrong with C++: The initial concept and the
implementation. (Bertrand Meyer)
--------
Maciej Katafiasz <email at mathrick.org>
http://mathrick.org