On Thu, Aug 07, 2014 at 09:44:39AM +0200, Vincent Lefevre wrote:
> On 2014-08-07 15:46:51 +1000, Peter Hutterer wrote:
> > whoah, this is pretty much a perfect example of how not to use goto...
> > adding three lines for error and exist isn't that hard, jumping from one
> > random spot into another random spot is not ok.
>
> This is a matter of taste. I've always disliked source code duplication
> in particular for the same task: when tracing, one too easily look
> at the wrong place, and when the code needs to be changed, it can
> too easily become inconsistent (e.g. one fixes an issue at one place
> and forgets the other ones).
>
> Note also that parseutils.c already has a similar use of goto.
parseutils.c has the accepted way of goto, which is summarised as
void foo() {
...
if (something goes wrong)
goto out;
....
return success;
out:
cleanup();
return error;
}
that's a common pattern in C and perfectly fine. Your patch jumps from one
else condition three levels in into another one one level in. that's not
ok.
I don't buy the duplication argument here either - all you need to do is
print an error and return. there's no complicated logic behind it.
Cheers,
Peter
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
