There might be good use cases for goto
,
but this is not one of them.
It's best when code reads from top to bottom.
The goto
statements break that readable flow.
Since the jump-to labels are quite far,
I'm forced to scroll to the end to see what they do.
After I scrolled, I find some code using variables defined at the beginning,
which I haven't memorized earlier,
so now I have to go back to top.
Effectively I'm making several jumps back and forth to understand how this works,
and then a few more jumps to verify carefully that it should work well.
This is a nightmare, and it doesn't have to be.
If you replace the goto
statements with the code at their jump-to labels,
the code becomes:
if (pipe(in)) {
return -1;
}
if (pipe(out)) {
close(in[0]);
close(in[1]);
return -1;
}
if (pipe(err)) {
close(out[0]);
close(out[1]);
close(in[0]);
close(in[1]);
return -1;
}
This is a lot more readable: everything I need to understand these guard statements are right there above.
You probably don't like the duplication when closing in
,
and again later in the code if fork
failed,
closing in
and out
.
It's true that code duplication is not good,
but I think it's the lesser of evils here,
and it's outweighed by the improvement in readability.
If you still disagree, and consider avoiding the duplication more important,
then you can improve this technique with goto
by using more descriptive labels,
for example:
close_err_and_out_and_in_and_return_with_failure:
close(err[0]);
close(err[1]);
close_out_and_in_and_return_with_failure:
close(out[0]);
close(out[1]);
close_in_and_return_with_failure:
close(in[0]);
close(in[1]);
return_with_failure:
return -1;
Avoid comments when possible. Instead of a comment, try to code in a way to make the comment unnecessary. For example:
/* This section is never be reached without a goto */
This comment could be replaced with this statement:
return pid;
Not only this is short and sweet,
it has the great benefit that since it's code,
it is enforced: the lines below it will not be reached,
no matter what changes in the code above the return
statement.
And we're kind of back to the pesky goto
statements.
Without the goto
statements,
this concern here about the comment and the logic wouldn't exist in the first place.