Hi Mathias,
* 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?
I meant your name and the date in particular. The rest of the comment is probably okay, though a FIXME would be helpful to indicate something not totally understood.
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.
Matters of style are always debatable, but in the Wine tests, we generally expect such things to succeed. As I noted elsewhere, a strange condition such as GetProcessHeap failing will result in many, many things failing, so being defensive in this case doesn't gain much except additional complexity in the tests. GetProcessHeap only fails if the Wine environment is not properly set up, and this really can never happen in the tests. When in doubt, read the source! MSDN isn't as helpful here.
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.
I'm sorry, I didn't note Vitaliy's prior response. I think we're both reacting to the defensiveness of your tests. Defensive programming in general may be a good thing, though there is some debate on this.. but I digress. In the Wine tests, if memory allocation begins to fail, many, many tests will fail, so the defensive style doesn't gain anything except additional code complexity. We wish to avoid that, so in general, we expect things like memory allocation to succeed in the tests.
So, to combine my advice and Vitaliy's: just don't check if memory allocation succeeds or fails in the tests. Assume it will succeed.
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.
No, you wouldn't notice any on a recent version of gcc, as it will silently accept C++ comments unless you specify -fpedantic or some such. The trouble is, not everyone uses a recent version of gcc, and their builds will fail with C++ comments left in. --Juan