Re: [RFC PATCH v1] tun: Cleanup error handling in tun_set_iff()

[Paul Moore <paul.moore@hp.com>]

On Thursday 06 August 2009 08:00:21 pm Herbert Xu wrote:
> On Thu, Aug 06, 2009 at 02:20:20PM -0400, Paul Moore wrote:
> > The code currently looks something like this:
> >
> > 	err = -ENOMEM;
> > 	buf = alloc(...);
> > 	if (!buf)
> > 		goto label;
> >
> > This means that in the common case where 'alloc()' completes without
> > error we are doing an extra, unnecessary assignment where we set the
> > value in 'err'. Now, if we change this slightly to match what I proposed
> > in the patch:
> >
> > 	buf = alloc(...);
> > 	if (!buf) {
> > 		err = -ENOMEM;
> > 		goto label;
> > 	}
> >
> > We eliminate that extra assignment in the case where 'alloc()' completes
> > without error, which should result in more efficient code (less
> > instructions in the common case).  Am I wrong?  If that is the case I
> > would appreciate an explanation ...
>
> Your style potentially introduces a second jump which may end
> up being worse compared to the extra work on a modern CPU.

Thanks, I hadn't thought of that possibility.  I suppose the impact of a 
second jump is going to depend quite a bit on the hardware it runs on 
(pipeline depth, branch prediction, etc.) and isn't as easy to quantify as I 
had hoped.  Oh well, I appreciate the explanation anyway :)

-- 
paul moore
linux @ hp