[PATCH] xkbcomp: Improved -w option parsing

[Peter Hutterer]

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