• Create Account

## goto considered harmful within global macros

21 replies to this topic

### #1swiftcoder  Senior Moderators

Posted 13 October 2013 - 07:41 AM

I was really quite surprised when I first attempted to add error checking at work, and my compilation failed with a error: use of undeclared label 'onError'.

From a ubiquitous global header in a (thankfully recently deceased) library within our project:

#define CheckSuccess(condition) do { \
if (!condition) { \
goto onError; \
} \
} while (0) 

The idea that an error checking macro would require the current function to implement a specifically named label for error cleanup (or, indeed, that there would be significant error cleanup not handled by RAII), is a little unpleasant to me...

I should probably add that the author of this particular gem thought that they were working around the lack of exception handling on our target platform.

Tristam MacDonald - Software Engineer @ Amazon - [swiftcoding] [GitHub]

### #2JTippetts  Moderators

Posted 13 October 2013 - 07:52 AM

Yikes.

### #3phantom  Members

Posted 13 October 2013 - 08:23 AM

Dear god o.O

### #4l0calh05t  Members

Posted 13 October 2013 - 08:35 AM

Meh, macros like this are common in C programs. Also popular:

#define CHECK(x) do { if(err = (x)) goto error; } while(0)

to save the actual error code (and assuming 0 = no error), which requires a variable of the appropriate type in addition to the proper goto label.

It should probably not be defined in a global header though.

Edited by l0calh05t, 13 October 2013 - 08:38 AM.

### #5Cornstalks  Members

Posted 13 October 2013 - 09:51 AM

I'm 100% surprised a velociraptor attack did not ensue. That macro only saves you from typing 7 keystrokes too... totally not worth using it IMO. And I hate macros like that for debugging and stepping through code.

Also, condition really should've been in parentheses (like in l0calh05t's post), because other wise things like CheckSuccess(false || true) would've resulted in madness.

[ I was ninja'd 71 times before I stopped counting a long time ago ] [ f.k.a. MikeTacular ] [ My Blog ] [ SWFer: Gaplessly looped MP3s in your Flash games ]

### #6SiCrane  Moderators

Posted 13 October 2013 - 09:54 AM

POPULAR

Really the thing that gets me most about this macro is that the name isn't in all caps. That's just wrong.

### #7MaxDZ8  Members

Posted 14 October 2013 - 12:53 AM

I feel something in my bowels. I'm going AFK a few minutes.

Previously "Krohm"

### #8Hodgman  Moderators

Posted 14 October 2013 - 01:12 AM

The real WTF is that other half of the error handling system isn't hidden behind an equivalent macro
#define CheckSuccess(condition) do { \
if (!condition) { \
goto onError; \
} \
} while (0)

#define OnError do { if(false) { \
onError: (void)0
#define EndError }} while (0) 
I mean, come on, if you're going to abstract away half of your error handling system behind your own invented keyword, at least be consistent and apply the same abstraction to the other half of the system too!

Edited by Hodgman, 14 October 2013 - 08:05 PM.

### #9Migi0027 (肉コーダ)  Members

Posted 14 October 2013 - 03:40 PM

The real WTF is that other half of the error handling system isn't hidden behind an equivalent macro

#define CheckSuccess(condition) do { \
if (!condition) { \
goto onError; \
} \
} while (0)

#define OnError do { if(false) { \
onError: (void)0
#define EndError }} while (0) 

WTF Indeed  .

FastCall22: "I want to make the distinction that my laptop is a whore-box that connects to different network"

Blog about... stuff (GDNet, WordPress): www.gamedev.net/blog/1882-the-cuboid-zone/cuboidzone.wordpress.com/

### #10Servant of the Lord  Members

Posted 16 October 2013 - 05:33 PM

On the rare occasions I need macros that are specific to a function or small set of related functions, I define them in the source file and then #undef them afterward. I've yet to put a goto in one though...

It's perfectly fine to abbreviate my username to 'Servant' or 'SotL' rather than copy+pasting it all the time.
All glory be to the Man at the right hand... On David's throne the King will reign, and the Government will rest upon His shoulders. All the earth will see the salvation of God.
Of Stranger Flames -

Posted 16 October 2013 - 05:45 PM

Pfft, I once ended a macro with an "else" (and nothing afterwards).

It was my first job though so I didn't know much better.

I had a load of check for joypad presses all over the code (bad I know). Then we got the standards guidelines through and pressing select and start for 3 seconds had to return to the menu. So I hacked it with a dodgy macro ;) That was when I used longjmp to return from anywhere back to the main menu too.

"Most people think, great God will come from the sky, take away everything, and make everybody feel high" - Bob Marley

### #12swiftcoder  Senior Moderators

Posted 16 October 2013 - 08:48 PM

That was when I used longjmp to return from anywhere back to the main menu too.

That's deliciously evil, but it's worth keeping in mind that it isn't unprecedented.

Your longjmp is effectively just implementing a call/cc, and would be considered a (fairly) sane method of control flow in most functional languages.

Tristam MacDonald - Software Engineer @ Amazon - [swiftcoding] [GitHub]

Posted 16 October 2013 - 08:59 PM

It's pretty similar to using exceptions (it resets the stack and jumps to the call to setjmp), without calling any destructors. It was C anyway, so that didn't matter.

Structured exception handling in Win32 uses setjmp/longjmp (but hides them behind macros).

EDIT: My evil macro was basically something which went before a call to GetPadState, something like this in pseudocode

macro CHECK_EXIT_GAME:

if(select and start held down for 3 seconds) longjmp MainMenuHandler

else

end macro

;)

Edited by Paradigm Shifter, 16 October 2013 - 09:03 PM.

"Most people think, great God will come from the sky, take away everything, and make everybody feel high" - Bob Marley

### #14swiftcoder  Senior Moderators

Posted 16 October 2013 - 09:02 PM

It's pretty similar to using exceptions (it resets the stack and jumps to the call to setjmp)

Aye, but "control flow via exceptions" gives everyone the heebee jeebees.

call/cc only typically causes heart attacks in procedural programmers

Tristam MacDonald - Software Engineer @ Amazon - [swiftcoding] [GitHub]

Posted 16 October 2013 - 09:17 PM

The funniest thing that happened in that project was when I had several bugs active at the same time:

I was running he NTSC version on a UK telly (so it was in black and white, TVs these days can handle NTSC and PAL). Not really a bug

There was a bug where the main menu music kept restarting after every data read

After a data read the CD speed was 2x so the music was way fast and high pitched (it was annoying steel drum band music as well!)

It was a golf game but something was wrong with the AI aiming so it was hitting the ball backwards instead of forwards

End result it was like watching a silent B&W golf comedy with mad double speed steel band music where the AI golfer kept hitting the ball backwards towards the camera.

"Most people think, great God will come from the sky, take away everything, and make everybody feel high" - Bob Marley

### #16wodinoneeye  Members

Posted 24 October 2013 - 06:17 AM

I was really quite surprised when I first attempted to add error checking at work, and my compilation failed with a error: use of undeclared label 'onError'.

From a ubiquitous global header in a (thankfully recently deceased) library within our project:

#define CheckSuccess(condition) do { \
if (!condition) { \
goto onError; \
} \
} while (0) 

The idea that an error checking macro would require the current function to implement a specifically named label for error cleanup (or, indeed, that there would be significant error cleanup not handled by RAII), is a little unpleasant to me...

I should probably add that the author of this particular gem thought that they were working around the lack of exception handling on our target platform.

I used to do it simpler...

#define CheckPrecondition(condition)  {  if ( ! condition) goto  ON_PRECONDITION_FAIL; }

Could have sworn I used to get that to work in a (native code) FSM macro simplified script language

--------------------------------------------Ratings are Opinion, not Fact

### #17Brother Bob  Moderators

Posted 24 October 2013 - 06:44 AM

I was really quite surprised when I first attempted to add error checking at work, and my compilation failed with a error: use of undeclared label 'onError'.

From a ubiquitous global header in a (thankfully recently deceased) library within our project:

#define CheckSuccess(condition) do { \
if (!condition) { \
goto onError; \
} \
} while (0) 

The idea that an error checking macro would require the current function to implement a specifically named label for error cleanup (or, indeed, that there would be significant error cleanup not handled by RAII), is a little unpleasant to me...

I should probably add that the author of this particular gem thought that they were working around the lack of exception handling on our target platform.

I used to do it simpler...

#define CheckPrecondition(condition)  {  if ( ! condition) goto  ON_PRECONDITION_FAIL; }

Could have sworn I used to get that to work in a (native code) FSM macro simplified script language

It is simpler but also prone to errors that the do-while(false) method is isn't. The do-while(false) method requires a semicolon to work, while it is optional and can silently change the behavior of your code whether it is there or not with your variant.

But, I'd say that such hacks deserves some happy debugging times when they do change the behavior...

### #18ultramailman  Prime Members

Posted 25 October 2013 - 11:51 AM

http://www.chiark.greenend.org.uk/~sgtatham/coroutines.html

Not exactly goto in macro, but switch case is similar to goto.

### #19swiftcoder  Senior Moderators

Posted 25 October 2013 - 12:26 PM

http://www.chiark.greenend.org.uk/~sgtatham/coroutines.html

Not exactly goto in macro, but switch case is similar to goto.

Please refer to the gem at the bottom of that page:
* PuTTY is a Win32 Telnet and SSH client. The SSH protocol code contains real-life use of this coroutine trick.
As far as I know, this is the worst piece of C hackery ever seen in serious production code.

Tristam MacDonald - Software Engineer @ Amazon - [swiftcoding] [GitHub]

### #20fastcall22  Moderators

Posted 25 October 2013 - 12:59 PM

TRTWTF is nobody escaped expression in parenthesis:

#define CheckSuccess(condition) do { \
if (!(condition)) { \
goto onError; \
} \
} while (0)

Hurr durr Ph'nglui mglw'nafh Cthulhu CheckSuccess(obj.frobnicate() || obj.bang()) R'lyeh wgah'nagl fhtagn and what not...

Edited by fastcall22, 25 October 2013 - 01:01 PM.

zlib: eJzVVLsSAiEQ6/1qCwoK i7PxA/2S2zMOZljYB1TO ZG7OhUtiduH9egZQCJH9 KcJyo4Wq9t0/RXkKmjx+ cgU4FIMWHhKCU+o/Nx2R LEPgQWLtnfcErbiEl0u4 0UrMghhZewgYcptoEF42 YMj+Z1kg+bVvqxhyo17h nUf+h4b2W4bR4XO01TJ7 qFNzA7jjbxyL71Avh6Tv odnFk4hnxxAf4w6496Kd OgH7/RxC