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,