Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

GCC 6: -Wmisleading-indentation vs. “goto fail;”

developerblog.redhat.com

Post reply on HN
Print view View original

21–30 of 168 posts

Re: GCC 6: -Wmisleading-indentation vs. “goto fail;”

#21

Post by AdmiralAsshat »

pif wrote: post #15
AdmiralAsshat wrote: post #6

Earlier quoted context omitted.

I'm in favor of braces around everything, but to be honest even I will occasionally use the indent if I only expect one action. e.g. if (true) foo(); else bar(); This looks prettier to my eyes than if(true){ foo(); } else { bar(); } However, I might not be the last person to touch the code. My coworker might come later and add: if (true) foo(); else bar(); baz(); And hence the indent problem.

Btw, I suppose you wanted to add baz() after bar(). Adding it after foo(), as you did, will not compile.

Thanks, I fixed it.

Why should the inability to write basic C preclude me from being allowed to pontificate about brace style like the greats?

Re: GCC 6: -Wmisleading-indentation vs. “goto fail;”

#22

Post by MichaelBurge »

gravypod wrote: post #2

This seems like it could have been avoided by people using braces around every block. Omitting braces in this case leads to a lot of problems.

Suppose Debian decides to hire you as their new release maintainer. They want to ensure that bugs such as the OpenSSL one mentioned in the article never happen again. What actions would you take before next month's release to attempt to catch it?

I see from your comment that you might hire a team of developers to go through a critical path of important C/C++ packages checking for style violations. Any code found missing braces will have a patch submitted upstream to correct the errant style. Any package that declines the style changes would be removed from Debian. Any developer you hire that misses style violations will be removed from the team.

Another approach might be: Turn on the warning mentioned in the article and manually review any cases that come up. That still might generate a lot of cases, but it at least seems possible.

Is there another actionable interpretation of your comment that I'm missing?

Re: GCC 6: -Wmisleading-indentation vs. “goto fail;”

#23

Post by hacker_9 »

AdmiralAsshat wrote: post #6
gravypod wrote: post #2

This seems like it could have been avoided by people using braces around every block. Omitting braces in this case leads to a lot of problems.

I'm in favor of braces around everything, but to be honest even I will occasionally use the indent if I only expect one action. e.g. if (true) foo(); else bar(); This looks prettier to my eyes than if(true){ foo(); } else { bar(); } However, I might not be the last person to touch the code. My coworker might come later and add: if (true) foo(); else bar(); baz(); And hence the indent problem.

I agree. But your last example won't compile anyway. Though this would:

  if (true)
    foo();
  else
    bar();
    baz();
But I have to admit if you ever actually do that, you have failed to fundamentally understand how your code is executed. No braces means one statement. Period.

In these cases languages which are auto formatted (C#, go-fmt, etc) likely have an advantage where these problems stick out more obviously.

Re: GCC 6: -Wmisleading-indentation vs. “goto fail;”

#24

Post by _0ffh »

AdmiralAsshat wrote: post #6
gravypod wrote: post #2

This seems like it could have been avoided by people using braces around every block. Omitting braces in this case leads to a lot of problems.

I'm in favor of braces around everything, but to be honest even I will occasionally use the indent if I only expect one action. e.g. if (true) foo(); else bar(); This looks prettier to my eyes than if(true){ foo(); } else { bar(); } However, I might not be the last person to touch the code. My coworker might come later and add: if (true) foo(); else bar(); baz(); And hence the indent problem.

In cases where I have only one statement I often drop the braces, but I also drop the newline.

  if (true) foo();
  else bar();
Not much room for a confused baz() here, or so I hope.

Re: GCC 6: -Wmisleading-indentation vs. “goto fail;”

#25

Post by mediocrejoker »

pavlov wrote: post #20
strommen wrote: post #13

Making braces optional in single-statement if/else/while/for clauses is one of the biggest anti-features in C. It's frustrating that it was ported forward to more modern languages like Java, JavaScript, C#, etc. I'm glad Python (with semantic whitespace) and Go (with gofmt) solve this problem.

Single-line ifs are pretty useful with traditional-style C libraries that expect all checks to be done at call site: if (ptr) call_oldschool_thingy(ptr);

I'm not sure that expanding your statement to multiple lines is any less "useful". I suppose in certain cases it can make code much more verbose, as one line effectively becomes 3-4, which can make it harder to navigate.

Re: GCC 6: -Wmisleading-indentation vs. “goto fail;”

#26

Post by epx »

gravypod wrote: post #2

This seems like it could have been avoided by people using braces around every block. Omitting braces in this case leads to a lot of problems.

This is the only item that I disagree with the Linux coding style ("Do not unnecessarily use braces where a single statement will do.")

Re: GCC 6: -Wmisleading-indentation vs. “goto fail;”

#27

Post by hacker_9 »

_0ffh wrote: post #24
AdmiralAsshat wrote: post #6

Earlier quoted context omitted.

I'm in favor of braces around everything, but to be honest even I will occasionally use the indent if I only expect one action. e.g. if (true) foo(); else bar(); This looks prettier to my eyes than if(true){ foo(); } else { bar(); } However, I might not be the last person to touch the code. My coworker might come later and add: if (true) foo(); else bar(); baz(); And hence the indent problem.

In cases where I have only one statement I often drop the braces, but I also drop the newline. if (true) foo(); else bar(); Not much room for a confused baz() here, or so I hope.

This is actually a worse solution. When speed reading code having control statements on their own lines is far more beneficial as you can at a glance, using the indentation, easily see what the flow of execution is.

Re: GCC 6: -Wmisleading-indentation vs. “goto fail;”

#28

Post by gravypod »

MichaelBurge wrote: post #22
gravypod wrote: post #2

This seems like it could have been avoided by people using braces around every block. Omitting braces in this case leads to a lot of problems.

Suppose Debian decides to hire you as their new release maintainer. They want to ensure that bugs such as the OpenSSL one mentioned in the article never happen again. What actions would you take before next month's release to attempt to catch it? I see from your comment that you might hire a team of developers to go through a critical path of important C/C++ packages checking for style violations. Any code found miss…

No. That is not what I mean. If you look at my response to iainmerric you will see that I don't feel you should change code that is working.

This type of bug is largely introduced from someone who is going in and changing a section of code. My philosophy is that if you are editing a section of code, you should update the braces in that section along with your current patch. Over time you see a larger and larger drop in these kinds of bugs as people always, on my team due to the habit of fixing braces and a matching drop from outside contributes due to a larger and larger portion of the code matching the style guide.

Re: GCC 6: -Wmisleading-indentation vs. “goto fail;”

#29

Post by asendra »

AdmiralAsshat wrote: post #6
gravypod wrote: post #2

This seems like it could have been avoided by people using braces around every block. Omitting braces in this case leads to a lot of problems.

I'm in favor of braces around everything, but to be honest even I will occasionally use the indent if I only expect one action. e.g. if (true) foo(); else bar(); This looks prettier to my eyes than if(true){ foo(); } else { bar(); } However, I might not be the last person to touch the code. My coworker might come later and add: if (true) foo(); else bar(); baz(); And hence the indent problem.

How about this one

(true) ? foo() : bar();

Re: GCC 6: -Wmisleading-indentation vs. “goto fail;”

#30

Post by andybak »

The argument for significant white space in Python goes as follows:

You need indentation for humans to understand the structure. Why do you also need braces for the parser to understand the structure when the parser can use the same information that your eyes use? You therefore avoid the possibility of the two signals contradicting each other.

Post reply on HN