goto considered harmful within global macros

Yikes.

Dear god o.O

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.

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.

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

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

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!

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 .

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...

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.

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.

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

;)

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

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.

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

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...

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

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

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.

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...