Hacker News Reader

Comment by Matt3o12_ | Hacker News Reader

This might be a bit off topic but I wonder why the vulnerability has been patched this way:

    if (!access_ok(VERIFY_WRITE, infop, sizeof(*infop)))
        goto Efault;

Why doesn’t the if use curly brackets? I thought it has been established that it is best practices to always use curly brackets even if they are explicit, especially after Apple's infamous goto bug[1].

Secondly, why does it use goto at all? I thought it has also been established not to use goto unless it is the only performant solution (and performance is important in that case). Sure Efault with probably kill the program but wouldn’t it still be better to use a function call considering that the desired resolution should be the same?

[1]: https://www.imperialviolet.org/2014/02/22/applebug.html

View on HN

1. Linux kernel coding style is documented here [1], and contains the line:

> Do not unnecessarily use braces where a single statement will do.

2. There is no built-in exception handling in C. `goto ERROR_HANDLING_CODE` is a common and well established pattern to handle exceptions in C, see e.g. [2].

[1] https://www.kernel.org/doc/html/v4.10/process/coding-style.h... [2] https://news.ycombinator.com/item?id=3883310

> I thought it has also been established not to use goto unless [...]

“Established” by whom? Certainly not by kernel developers — `goto` is very common in all kernels I have looked at (xnu, Linux, bsd)

And it has quite a consensus as the best solution to this problem. Goto being considered harmful is a generally true statement. However, this usage is a more specific exception to the rule, with objective benefits vs alternatives.

"goto considered harmful" is an old meme from 50 years ago, when control blocks like if and for were invented.

From your link: Maybe the coding style contributed to this by allowing ifs without braces, but one can have incorrect indentation with braces too, so that doesn't seem terribly convincing to me.

By the way, gcc now has -Wmisleading-indentation, which is activated by default if you enable -Wall: https://news.ycombinator.com/item?id=10875449

The kernel style guide mandates no curly brackets in this case.

https://www.kernel.org/doc/html/v4.10/process/coding-style.h...

Note: The present solution replaced the goto with a return:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...

goto is kind of a necessity in any complex C codebase unless you want to duplicate tons of code. Sometimes you need to jump way out of the context, especially to handle errors, and C does not have "exceptions" (although you could do them with setjmp/longjmp)

goto is the best way to do early exit and cleanup from a C function. The alternatives are 1) deeply nested if's, one level for each function call that can return an error code, or 2) repeat the same cleanup code over and over again at every exit point.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...

Reply on news.ycombinator.com