Share via

I hate magic

  • Article

C++ magic in particular. What's wrong with this code?

#define BLANK_IF_NULL(_s_) ((_s_) ? (_s_) : (TEXT("")))

We had a macro like this in MSN Explorer that would ensure you didn't pass a NULL string around. Well there's lots of things wrong with the code - the standard _s_ will get evaluated multiple times problem like the classic min/max macros - for that reason alone it should have been an inline function. But can you believe this piece of code was one of our top crash buckets from the Watson data we received from the first Beta release of MSN Explorer. To figure out why you need to actually "use" the macro a bit.

CComBSTR sbstrSomeString;

// ... some real code went here

// variable name changed to indicate the reality of the situation
LPCWSTR pszUnfortunateAlias = BLANK_IF_NULL(sbstrSomeString);

// ... even more real code went here

SomeFunction(pszUnfortunateAlias);

So naturally this would blow up. Sometimes. The magic of temporary variables. When the macro is expanded the type "returned" becomes a CComBSTR. When sbstrSomeString evaluates to NULL a temporary CComBSTR was created. And the value was assigned to pszUnfortunateAlias. Then the temporary is destroyed because it goes out of scope. So now we are in what I like to refer to as "roll the dice" mode. We got lucky a lot of the time and the memory was not reallocated. However, luck like that only goes so far...

Comments

  • Anonymous
    February 03, 2005
    The comment has been removed
  • Anonymous
    February 03, 2005
    The reason CComBSTR allows an implicit class is of course for other magic purposes so you can do something like this:

    SetWindowTextW(hwnd, sbstrSomeString);

    I'm not saying I disagree with you - that's just the way it goes, eh?