Shell32 File Property Dialog

Andreas Mohr andi at rhlx01.fht-esslingen.de
Wed Nov 2 08:17:12 CST 2005


Hi,

On Wed, Nov 02, 2005 at 01:15:32AM +0100, Johannes Anderwald wrote:
> Andreas Mohr wrote:
> >>            FIXME("Unhandled Verb %xl\n",LOWORD(lpcmi->lpVerb));
> >
> >What kind of format specifier is that supposed to be?
> >I don't know that one...
> >Maybe use %p instead? (or %lx??)
> This statement should be %lx. However, this statement is _not_ part of 
> my patch.
Doh, sorry, didn't realize that.

> >>   dwFileLo = GetFileSize(hFile, &dwFileHi);
> >>   CloseHandle(hFile);
> >>
> >>   if (dwFileLo == 0xFFFFFFFF && GetLastError() != NO_ERROR) 
> >>       return FALSE;
> >
> >This whole check sounds very weird.
> >Why are you checking with NULL hiword when there might be a > 4GB file?
> >(and to make it worse, even one with e.g. a size of 0x12345678FFFFFFFF 
> >!!!!!!)
> >And directly after that even doing a full large file check **again**?
> >Not to mention that you're not using the INVALID_FILE_SIZE macro that I 
> >really,
> >really think should be used since it's been created *exactly* for this 
> >purpose
> >(and for the even better purpose of *never* managing to write/edit/delete 
> >a
> >0xFFFFFF or 0xFFFFFFF instead...)
> 
> I agree that the INVALID_FILE_SIZE should be used. However, 
> INVALID_FILE_SIZE macro is exactly 0xFFFFFFFF. Furthermore, this code 
> does work with file sizes of 0x12345678FFFFFFFF. Have a look at the MSDN 
> documentation[1]. Alternatively, GetFileSizeEx could be used.
My point was that using INVALID_FILE_SIZE as intended by the system *ensures*
that you may never potentially mis-write or mis-modify 0xffffffff.

And yes, I had a look at the MSDN page, incidentially that's exactly why
I was commenting on it since I knew how horrible GetFileSize() is...
And it appears your code was (almost) right indeed, minus the problem that
you're calling GetLastError() after CloseHandle() (which will also modify
last error on failure).
But you probably intended to call CloseHandle() before the check in order
to avoid an extra CloseHandle() in the code...

Argh, whoever doesn't hate this particular API please raise your hands now...

Thanks for your work!

Andreas



More information about the wine-devel mailing list