Juan Lang schreef:
Hi Maarten,
overall this patch looks pretty good. A few minor comments:
tempbuffer = HeapAlloc(GetProcessHeap(), 0, TEMP_BLOCK_SIZE);if (!tempbuffer)goto out;You return TRUE in this case. You probably ought to set last error and return FALSE instead.
Oops, thanks for catching that. I think HeapAlloc will already set the last error correctly, so I don't have to worry about that.
while (ReadFile(hFile, tempbuffer, TEMP_BLOCK_SIZE,&readbytes, NULL) && readbytes)
CryptHashData(hash, tempbuffer, readbytes, 0);Why not while ((ret = ReadFile...) && readbytes) instead? This way you can at least return failure if reading the file fails for whatever reason.
I'll fix that too, thanks.
- SetLastError(ERROR_INSUFFICIENT_BUFFER); return TRUE;
Why are you setting ERROR_INSUFFICIENT_BUFFER in the success case? If it's just to pass that test, I think you might remove the test instead: in general we don't like to check last error in case of success, unless we know an app does so.
It is in the success case where no buffer is allocated (eg it wants to know the buffer size). It might be better to just wrap it in an 'else { ... }' to be more specific about this.
Cheers, Maarten.