2

I have a function-like macro that takes in an enum return code and a function call.

#define HANDLE_CALL(result,call) \
    do { \
        Result callResult = call; \
        Result* resultVar = (Result*)result; \
        // some additional processing
        (*resultVar) = callResult; \
    } while(0)

Does fancy pointer casting to Result* and subsequent de-referencing gain you anything? That is, is there any advantage to doing this over just:

callResult = call;
// additional processing
*result = callResult;

The macro is used like this:

Result result;
HANDLE_CALL(&result,some_function());

By the way, this isn't my code and I'm not a power C user so I'm just trying to understand if there is any logic behind doing this.

tskuzzy
  • 35,812
  • 14
  • 73
  • 140
  • @SteveJessop: Sorry, result in the macro is a `Result*`. I got confused by the naming. I hope everything is syntactically correct now. – tskuzzy Jun 12 '12 at 16:59

3 Answers3

4

I think what it gives you, is that the user of the macro can pass in a pointer of any old type, not just Result*. Personally I'd either do it your way, or if I really wanted to allow (for example) a void* macro argument I'd write *(Result*)(result) = callResult;.

There's another thing it might be, depending what the rest of the macro looks like. Does "some additional processing" mention resultVar at all, or is the line (*resultVar) = callResult; conditional? If so, then resultVar exists in order to ensure that the macro evaluates each of its arguments exactly once, and therefore behaves more like a function call than it would if it evaluated them any other number of times (including zero).

So, if you call it in a loop like HANDLE_CALL(output++, *input++) then it does something vaguely predictable. At least, it does provided that output is a Result* as the macro author intended. I'm still not sure what the cast gives you, other than allowing different argument types like void* or char*.

There are some situations where it could make another difference whether you have that extra cast or not. For example, consider:

typedef double Result;

int foo() { return 1; }
int i;
HANDLE_CALL(&i, foo());

if typedef double Result; isn't visible on screen, the other three lines appear pretty innocuous. What's wrong with assigning an int to an int, right? But once the macro is expanded, bad stuff happens when you cast an int* to double*. With the cast, that bad stuff is undefined behavior, most likely double is bigger than int and it overruns the memory for i. If you're lucky, you'll get a compiler warning about "strict aliasing".

Without the cast you can't do double* resultVar = &i;, so the original macro with that change catches the error and rejects the code, instead of doing something nasty. Your version with *result = callResult; actually works, provided that double can accurately represent every value of int. Which with an IEEE double and an int smaller than 53 bits, it can.

Presumably Result is really a struct, and nobody would really write the code I give above. But I think it serves as an example why macros always end up being more fiddly than you think.

Steve Jessop
  • 273,490
  • 39
  • 460
  • 699
1

As Steve says, the cast is to be able to pass something different than Result* to the macro. But I think it just shouldn't do the cast, this is dangerous, but only the initialization. Better would be

#define HANDLE_CALL(result,call)         \
    do {                                 \
        Result callResult = (call);      \
        Result* resultVar = (result);    \
        /* some additional processing */ \
        (*resultVar) = callResult;       \
    } while(0)

this would enforce that result is assignment compatible to Result*, so it could either be Result* or void*. All other uses to force-cast a (pointer to a larger structure) to (its first field of type Result) or so, are useless playing with fire.

Jens Gustedt
  • 76,821
  • 6
  • 102
  • 177
0

It all depends on the daft things people pass in to the instantiations. Do you really need a macro - an inline function would be better.

Julian
  • 1,522
  • 11
  • 26
  • 7
    I imagine since he is asking why the macro does something, that he didn't write it. – Hunter McMillen Jun 12 '12 at 16:56
  • I'm sure you are right, but since he is asking, I expect he is prepared to make changes to it. do { } while(0) is pretty redundant too relative to { } – Julian Jun 12 '12 at 17:01
  • 1
    @Julian: See http://stackoverflow.com/questions/154136/why-are-there-sometimes-meaningless-do-while-and-if-else-statements-in-c-c-mac. – Oliver Charlesworth Jun 12 '12 at 17:03