gdi32: "StretchDIBits" with value of zero for "xSrc" and "ySrc"

Juan Lang juan.lang at gmail.com
Tue Sep 23 16:26:24 CDT 2008


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



More information about the wine-devel mailing list