Hello Juan! Thanks for your valuable comments.
* mkosch (2008/09/22)
Please don't put such comments in source files, the source files aren't a changelog.
Is there a right place for such comments? Since I didn't yet prove if this code is buggy and also don't want to investigate further time in this (unless practical problems occur for me) Bugzilla is the wrong place. (Note: I think there should be problems i.e. with XOR, because this code appears to XOR the destination bitmap with gaps and then again with the destination bitmap, leaving the gaps.)
- hProcessHeap = GetProcessHeap();
- assert(hProcessHeap != NULL);
This assert is pointless, GetProcessHeap cannot reasonably fail. And more generally..
According to MSDN [1] there is a chance that "GetProcessHeap" might fail. It's bad style to expect what doesn't always have to happen.
- assert(hDC != NULL);
dont' use assert() in the tests, especially for conditions such as this which have nothing to do with the function you're testing. [...] or use ok to check them if there's some reasonable probability they may fail
Vitaliy Margolen responded to my previous version of this patch:
if (!pBitmapData)
ok(0, "Memory allocation failed.\n");
Don't do this. If allocation failed assert or skip the test entirely.
I'm a little confused now, because he told me to not use "ok" but suggested to assert.
So either assume that such basic functions will succeed,
I may do this for something less important like a test case, although I still think it's not good style.
or use a combination of skip and return if one fails.
I will try once more, this time using "skip" instead of assertions.
In the second place, C++-style comments are not allowed in Wine source.
Sorry I missed this one out. I didn't notice any compiler warnings.
[1] http://msdn.microsoft.com/en-us/library/aa366569(VS.85).aspx