Spot the bug – control flow macro

Found this in a code review.

Well, in all candor, I didn't find it; our automated code review tool found it.  But I'm taking credit for it because I read the output of the automated code review tool. :-)

Code rewritten because I'm too lazy to look it up.

// macros.h
#define ENSURE_EQUAL(x, y) if (x != y) { bail(); }

// implementation.cpp
hr = foo();
ENSURE_EQUAL( foo_supported() ? S_OK : E_FOO_NOT_SUPPORTED, hr );

Comments (4)

  1. Barbie says:


    if (foo_supported() ? S_OK : (E_FOO_NOT_SUPPORTED != hr)) …

  2. Mark says:

    As always, missing parentheses.  I think this should become a preprocessor warning.

  3. Aaron says:

    How did the code review tool catch it?  What criterion did it invoke?

  4. PREfast (…/ms933794.aspx) caught this error, sort of by mistake.

    The type of the (a ? b : c) expression is the common type of b and c; if b and c are of different types then an attempt is made to implicitly convert c to the same type as b.

    PREfast saw the (foo_supported() ? S_OK : (E_FOO_NOT_SUPPORTED != hr)) expression and said "hmm, b is an HRESULT and c is a boolean value.  Yes, there's an implicit conversion from boolean to HRESULT but it merits a warning."

    I saw the "boolean being implicitly converted to HRESULT" warning on the ENSURE_EQUAL(…) line, was confused, and dug further until I realized what was going on.

Skip to main content