New EnumPrinterDataEx

Ian Pilcher pilcher at concentric.net
Mon Feb 12 21:26:40 CST 2001


Dmitry Timoshkov wrote:
> 
> Certainly RegCloseKey and HeapFree can fail if someone will feed garbage to them,
> but if you really know that parameters you pass are valid, there is no reason to
> check the returned values. Please use simple RegCloseKey and HeapFree instead of
> monster macros.

This is certainly something I didn't expect.  I don't want to get into a
religious war over coding style, but I'm not comfortable ignoring the
return value of a function that can fail -- no matter how unlikely that
failure is.

  - I do not really know that the parameters I am passing are valid. OK,
    I'm pretty darn sure in this case, but I guarantee that the day I
    make that assumption will be the day I pass the wrong variable or
    make some similarly stupid mistake.

  - My understanding is that the goal of the Wine project is to reverse
    engineer the Win32 API.  If this is true, the Win32 API documen-
    tation is the "contract" between the author of a function (such as
    HeapFree) and the caller of that function.  Since the documentation
    does not mention the circumstances under which the call will fail,
    assuming that it doesn't fail is relying on an undocumented
    behavior.  (Even if I examine the source code of the Wine functions
    that I call and ensure that the current implementations cannot fail
    when I call them, there is no guarantee that this behavior will not
    change in the future.)

  - Consider the actual impact of this check.  In each case it's a
    comparison of a 32-bit return value (presumably a register on all
    platforms) to 0.  Assuming a successful call to the ASCII version of
    this function, this is a total of six comparisons to 0.  If Wine is
    compiled with warnings disabled, the compiler should optimize out
    the comparisons entirely.

As to "monster" macros, I could have written each in one or two lines,
but I have found that C constructs of the "if (!(r = function x)))" form
cause me an inordinate amount of trouble when I have to analyze them
later.  (Note that I am *not* telling other people not to use such
constructs.  I'll respect your coding style, but please grant me the
same courtesy.)  BTW, I chose macros rather than inline functions so
that any warnings generated would show the correct function name.

I hope this clarifies my reasoning somewhat.  Of course it's not
ultimately my decision what gets committed, but I think that the
arguments for checking the return value are compelling, and I think that
the use of macros greatly increases the readability of the code.

Thanks!
-- 
========================================================================
Ian Pilcher                                       pilcher at concentric.net
========================================================================



More information about the wine-devel mailing list