Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: question on common error-handling idiom
Original-Message-ID: <Pine.LNX.4.58.0411041140410.2187@ppc970.osdl.org>
Date: Thu, 4 Nov 2004 20:06:51 GMT
Message-ID: <fa.gut7vu7.8imp9l@ifi.uio.no>

On Tue, 2 Nov 2004, Chris Friesen wrote:
>
> While nice to read, it would seem that it might be more efficient to do the
> following:
>
> if (error condition) {
> 	err = -ERRORCODE;
> 	goto out;
> }
>
>
> Is there any particular reason why the former is preferred?  Is the compiler
> smart enough to optimize away the additional write in the non-error path?

Quite often, the additional write is _faster_ in the non-error path.

If it's a plain register, the obvious code generation for the above is

		..
		testl error-condition
		je no_error
		movl $-XXX,%eax
		jmp out;
	no_error:
		...

which is a lot slower (for the common non-error case) than


		movl $-XXX,%eax
		testl error-condition
		jne out

because forward branches are usually predicted not-taken when no other
prediction exists.

You can do the same thing with

	if (unlikely(error condition)) {
		err = -ERRORCODE;
		goto out;
	}

which is hopefully even better, but the fact is, the regular

	err = -ERRORCODE;
	if (error condition)
		goto out;

is just _smaller_ and simpler and quite often more readable anyway.

It has the added advantage that it tends to stylistically match what I
consider the proper error return behaviour, ie

	err = function_returns_errno(...)
	if (err)
		goto out;

ie it looks syntactically identical when "error condition" and the error
value happen to be one and the same. Which is, after all, _the_ most
common case.

So I personally tend to prefer the simple format for several reasons.
It's small, it's consistent, and it maps well to good code generation.

The code generation part ends up being nice when something goes wrong.
When somebody sends in an oops, I often end up having to look at the
disassembly (and no, a fancy debugger wouldn't help - I'm talking about
the disassembly of the "code" portion of the oops itself, and matching it
up with the source tree - the oops doesn't come with the whole binary),
and then having code generation match the source makes things a _lot_
easier.

Does it make sense for other projects? Dunno. But it is, as you noted, a
common idiom in the kernel. Getting used to it just makes it even more
readable.

			Linus