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

41–50 of 168 posts

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

#41

Post by Grue3 »

Piskvorrr wrote: post #16
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.

Prettier but potentially dangerous? That's the core of the problem, I believe.

Everything is potentially dangerous. I mean, pointers, anyone? So many things that can go wrong with those.

And besides, code is for humans to read and only incidentally for computers to execute. Might as well optimize for prettiness.

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

#42

Post by nathanielc »

deweerdt wrote: post #36
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.

I think that gofmt is particularly innovative, in the sense that it acknowledges that formatting is integral part of the language. In the sense that a programming language is not only made to be parsed by a computer, but also read back by a human.

Yes, and that the nuances of how things are formatted are really not that important, but rather having a standard which provides a consistent reading experience is the important aspect.

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

#43

Post by return0 »

bryanlarsen wrote: post #34
pavlov wrote: post #20

Earlier quoted context omitted.

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);

Braces don't prevent you from using single-line if: if (ptr) { call_oldschool_thingy(ptr); }

many people use specific styles for where to place { and } (e.g in a line by itself) so this might look 'ugly'

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

#44

Post by GFK_of_xmaspast »

pif wrote: post #17

> it’s been finding real world bugs One more case to support -Werror.

-Werror holds things back, because it makes the gcc maintainers hesitant to add more warnings on grounds of "it will break old code that uses -Werror", and in fact I was surprised to see that this warning is going into -Wall.

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

#45

Post by gravypod »

pkaye wrote: post #39
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 remember doing this mistake back in college. Was required to program and demo the B-Tree backend for a mini database. Things were very well tested for tons of random sets of data but while waiting for the my turn to demo, added one line of debug print on an "if" statement which caused the now dangling statement to be always executed. This caused a subtle bug that the professor caught in his testing. Sucked because…

We have all learned that lesson one way or another. It's never fun.

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

#46

Post by return0 »

lmm wrote: post #37
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.

Extra braces add visual clutter and reduce readability, especially when they mean a function no longer fits on one screen. So if there's a way to eliminate that kind of bug without having to add more braces then I'm all in favour of it. (Personally my preference would be a linter that automatically runs on checkin, and refuses commits that do not conform to the style guide)

> add visual clutter and reduce readability

one person's clutter is another person's markers. I wouldn't agree they reduce readability

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

#47

Post by gravypod »

kllrnohj wrote: post #31
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.

People will make mistakes. Policy does not prevent that. Tools do.

Not in every case. I'd say that policy does help in some cases and tools do in others.

What I often see is people just ignore the output from tooling and commit anyway.

No method of prevention is perfect, but everything you do can help to increase stability in projects.

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

#48

Post by frandroid »

return0 wrote: post #43
bryanlarsen wrote: post #34

Earlier quoted context omitted.

Braces don't prevent you from using single-line if: if (ptr) { call_oldschool_thingy(ptr); }

many people use specific styles for where to place { and } (e.g in a line by itself) so this might look 'ugly'

Nothing prevents them from not using single-line ifs.

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

#49

Post by mikeash »

gravypod wrote: post #28
MichaelBurge wrote: post #22

Earlier quoted context omitted.

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…

That seems like a somewhat different case, though. Your approach will help prevent new bugs like this from being introduced, but won't help you against existing examples of it in areas you're not currently working on. And just because it's been around a long time doesn't mean it's "working." Goto fail showed that the bugs can be subtle and easily missed for a long time.

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

#50

Post by KMag »

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);

Both Rust and Golang have optional parens and mandatory braces, so for the same number of keystrokes one would have something more like:

    if ptr { call_oldschool_thingy(ptr) }
(Keystroke count isn't an important metric for me, but apparently very important to some people.)
Post reply on HN