4 ms·

Popularity doesn't make a bad idea any better.

asveikau 10y ago
Sure, but you know what does make this idea better? The fact that it's pretty much the most sane way to consistently handle errors in a complicated C program. In particular if you have multiple paths out of your function and it also makes memory allocations, that's an overcomplicated amount of free() calls you need to take care of, remember are there, and maintain as you add/remove allocations and failure paths. goto lets you keep it simple by only maintaining a single block of free()s at your function's single exit path. Granted it seems this file is C++ and therefore the "going full RAII" option exists and could avoid goto in favor of exceptions or early returns... But if your code is C style that is a tough conversion to make, and not everybody buys into it equally.
- Sean1708 10y ago
  But you could also do all of that without omitting curly brackets.
- Piskvorrr 10y ago
  True, insightful, but completely beside the point - which was "bracketless conditionals open up an avenue for coding errors." The fail in #gotofail wasn't in using GOTO - it was a coding error, where a misindented line after a bracketless condition was always executed, when it should have been only executed inside the condition.
  - cyphar 10y ago
    You're conveniently not mentioning that this would cause very similar issues: if (some_cond) { goto fail; } goto fail; if (some_cond) { goto fail; } if (some_cond) { goto fail; } if (some_cond) { goto fail; } Is it more obvious? Sure. But the real issue is that they weren't testing all of the important failure cases, not that they weren't using one code style over another. Coding style is not going to help you if one of those some_cond values is incorrectly written...
digi_owl 10y ago
When it shows up in the kernel it is likely to be deep in a hairy if cluster inside some driver code. Thus if you get a failure of some sort it is quite likely that it is best to just drop everything and get out, as the whole edifice is about to come down anyways.
- Piskvorrr 10y ago
  if (something) { blah(); goto fail; } ^^^ that part works if (something) blah(); goto fail; ^^^ that part is written in a misleading way and doesn't do what it seems to do on first glance. It is unfortunate that the problematic line contained a jump, because "#gotofail" seems to convey "GOTO considered harmful", while it's something completely unrelated. Ironic, really, given the actual meaning of "human looks at a block of text and misparses it."
  - cyphar 10y ago
    You're conveniently not mentioning that this would cause very similar issues: if (some_cond) { goto fail; } goto fail; if (some_cond) { goto fail; } if (some_cond) { goto fail; } if (some_cond) { goto fail; } Is it more obvious? Sure. But the real issue is that they weren't testing all of the important failure cases, not that they weren't using one code style over another. Coding style is not going to help you if one of those some_cond values is incorrectly written...
    - Piskvorrr 10y ago
      "More obvious" is precisely my point: the misindentation in your example jumps out at anyone familiar with the syntax. This is not an universal cure, and won't help you with other types of semantic errors: nothing is going to help you if you try to avoid thinking and rely solely on your tools; that is the essence of cargo-cult programming. OTOH, this tool will help you avoid a common pitfall with potentially disastrous consequences. A high-visibility vest for coding, if you will: won't save you from slipping or your head from falling objects, but neither is its purpose. (Iterative bootstrapping is how we got from sharpened rocks to virtual machines, not by "meh, this minor improvement won't fix everything, so why bother.")
loeg 10y ago
Popularity may suggest that it is not a bad idea.
- Piskvorrr 10y ago
  May or may not. Given that popularity breeds more popularity, all it actually says is that it was popular enough, sometime in the past, to start a runaway process (I don't really want to start a flame war of "look at those popular yet fairly horrible things/languages/whatevers")

Sure, but you know what does make this idea better? The fact that it's pretty much the most sane way to consistently handle errors in a complicated C program. In particular if you have multiple paths out of your function and it also makes memory allocations, that's an overcomplicated amount of free() calls you need to take care of, remember are there, and maintain as you add/remove allocations and failure paths. goto lets you keep it simple by only maintaining a single block of free()s at your function's single exit path. Granted it seems this file is C++ and therefore the "going full RAII" option exists and could avoid goto in favor of exceptions or early returns... But if your code is C style that is a tough conversion to make, and not everybody buys into it equally.

Sean1708 10y ago
But you could also do all of that without omitting curly brackets.
Piskvorrr 10y ago
True, insightful, but completely beside the point - which was "bracketless conditionals open up an avenue for coding errors." The fail in #gotofail wasn't in using GOTO - it was a coding error, where a misindented line after a bracketless condition was always executed, when it should have been only executed inside the condition.
- cyphar 10y ago
  You're conveniently not mentioning that this would cause very similar issues: if (some_cond) { goto fail; } goto fail; if (some_cond) { goto fail; } if (some_cond) { goto fail; } if (some_cond) { goto fail; } Is it more obvious? Sure. But the real issue is that they weren't testing all of the important failure cases, not that they weren't using one code style over another. Coding style is not going to help you if one of those some_cond values is incorrectly written...

digi_owl 10y ago

When it shows up in the kernel it is likely to be deep in a hairy if cluster inside some driver code. Thus if you get a failure of some sort it is quite likely that it is best to just drop everything and get out, as the whole edifice is about to come down anyways.

Piskvorrr 10y ago
if (something) { blah(); goto fail; } ^^^ that part works if (something) blah(); goto fail; ^^^ that part is written in a misleading way and doesn't do what it seems to do on first glance. It is unfortunate that the problematic line contained a jump, because "#gotofail" seems to convey "GOTO considered harmful", while it's something completely unrelated. Ironic, really, given the actual meaning of "human looks at a block of text and misparses it."
- cyphar 10y ago
  You're conveniently not mentioning that this would cause very similar issues: if (some_cond) { goto fail; } goto fail; if (some_cond) { goto fail; } if (some_cond) { goto fail; } if (some_cond) { goto fail; } Is it more obvious? Sure. But the real issue is that they weren't testing all of the important failure cases, not that they weren't using one code style over another. Coding style is not going to help you if one of those some_cond values is incorrectly written...
  - Piskvorrr 10y ago
    "More obvious" is precisely my point: the misindentation in your example jumps out at anyone familiar with the syntax. This is not an universal cure, and won't help you with other types of semantic errors: nothing is going to help you if you try to avoid thinking and rely solely on your tools; that is the essence of cargo-cult programming. OTOH, this tool will help you avoid a common pitfall with potentially disastrous consequences. A high-visibility vest for coding, if you will: won't save you from slipping or your head from falling objects, but neither is its purpose. (Iterative bootstrapping is how we got from sharpened rocks to virtual machines, not by "meh, this minor improvement won't fix everything, so why bother.")

loeg 10y ago

Popularity may suggest that it is not a bad idea.

Piskvorrr 10y ago
May or may not. Given that popularity breeds more popularity, all it actually says is that it was popular enough, sometime in the past, to start a runaway process (I don't really want to start a flame war of "look at those popular yet fairly horrible things/languages/whatevers")