On Tuesday 22 January 2008 01:04:31 am Detlef Riekenberg wrote: > - gdi32: Make GetICMProfile behave more like native. Rewrite ansi > version as a wrapper and move color management functions to their own > file. > > The large subject is an indicator, that the Patch can be splitted. > IMHO, moving the stubs to a different file can be a seperate patch.
My reasoning was that moving the code and rewriting the ansi version are neutral operations from a functional perspective, and the tests show that (actually, the previous implementation crashes on my test, but you get the point). I agree that the changelog entry is long, I could have left the second sentence out since it's more of a patch reading hint. > > +BOOL WINAPI GetICMProfileA(HDC hdc, LPDWORD size, LPSTR filename) > > + if (!hdc || !size || !filename) return FALSE; > > This is not the same as documented in MSDN, and > > > +BOOL WINAPI GetICMProfileW(HDC hdc, LPDWORD size, LPWSTR filename) > > + > > + if (!hdc || !size) return FALSE; > > ANSI and UNICODE are different. > Are you really sure? > (Please insert a big warning in the comment then) Yes, I'm sure, it's what the tests brought up and they serve very well as documentation here IMO. > Returning FALSE without a SetLastError() looks not correct, compared to > SetLastError(ERROR_INSUFFICIENT_BUFFER) below. We return FALSE without a SetLastError() in many places. I think the consensus is that we do it only when applications depend on it. Setting ERROR_INSUFFICIENT_BUFFER is part of the idiom for querying the buffer size, clearly something that applications can be expected to depend on. -Hans
