[coreboot] Re: More coding style change proposals
On Tue, Jun 25, 2019 at 1:29 PM Julius Werner wrote:
> No we don't. We had a long discussion about multi-line comment styles
> some years back and I made the same argument there. ;)
you're totally right, oops! I misread the coding style again just a
few days ago. Sheesh!
>. I'd
> bet 95+% of coreboot systems run Linux afterwards, and when adding
> support for new platforms it's just normal that the hardware vendor
> has the same people working on both kernel and firmware drivers for
> the same component.
I'm sure you are absolutely correct here. That said, I'm starting to
encounter folks from, e.g., the Rust community who find some aspects
or our coding style odd. I'm expecting to see more of this. From
personal experience, I can say that moving from Rust-based
hypervisors, kernels, and firmware to coreboot is starting to feel a
bit ... weird, and in this case, coreboot is the outlier.
But yeah, point taken that many people are working in the Linux kernel
style nowadays ... OTOH, we have had our encounters with UeFi SyLe
CoDe, and they could make a similar argument around code style ...
I'm still of the opinion that the addition of {} after if is a good
idea. After years of Go and now Rust, seeing those unprotected bits of
code after an if just scares me.
Thanks
ron
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: More coding style change proposals
Just out of curiosity, I checked the Google C++ style guide, but it
doesn't really help:
> In general, curly braces are not required for single-line statements, but
> they are allowed if you like them;
When I first started doing firmware, I was kind of annoyed having to
go back and remove the braces from single line statements. It's kind
of muscle memory to add them. I also don't have to think up front if I
need them or not. And to Vadim's point, adding debug statements into a
block is less error prone and less work if the braces are already
there. For me, reading a diff where debug prints are added is easier
if the braces were already there. Then I don't have to concern myself
that I changed the condition.
e.g.,
> /* Default to 4 KiB search space. */
> - if (req_size == 0)
> + if (req_size == 0) {
> + debug("Using default size\n");
> req_size = 4 * 1024;
> + }
>
vs
> /* Default to 4 KiB search space. */
> if (req_size == 0) {
> + debug("Using default size\n");
> req_size = 4 * 1024;
> }
But at the same time, I've grown accustomed to reading code like this:
> if (table == NULL)
> return -1;
I guess I would opt for requiring braces.
On Tue, Jun 25, 2019 at 3:17 PM Vadim Bendebury wrote:
>
>
>
> On Tue, Jun 25, 2019 at 1:29 PM Julius Werner wrote:
>>
>> The improvement of requiring a { on the ifs is known to
>> > have positive impact; it's why Rust and Go both require it to my
>> > understanding.
>>
>> How is this actually "known"?
>
>
> Mandatory brackets are especially helpful when one makes quick debugging
> changes, say adding a print statement or an assignment and forgets to add the
> brackets to single statement clauses.
>
> The compiler option would be useless if the debug code does not follow the
> indentation pattern.
>
> -vb
>
> ___
> coreboot mailing list -- coreboot@coreboot.org
> To unsubscribe send an email to coreboot-le...@coreboot.org
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: More coding style change proposals
On Tue, Jun 25, 2019 at 1:29 PM Julius Werner wrote:
> The improvement of requiring a { on the ifs is known to
> > have positive impact; it's why Rust and Go both require it to my
> > understanding.
>
> How is this actually "known"?
Mandatory brackets are especially helpful when one makes quick debugging
changes, say adding a print statement or an assignment and forgets to add
the brackets to single statement clauses.
The compiler option would be useless if the debug code does not follow the
indentation pattern.
-vb
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: More coding style change proposals
> we mandate comments as follows:
> /*
> * something
> */
No we don't. We had a long discussion about multi-line comment styles
some years back and I made the same argument there. ;) That's why our
coding style now allows two versions so you can pick whatever suits
the situation at hand better:
/*
* long
* form
*/
/* short
form */
> I note that the changing our comment style would have zero impact on
> code safety. The improvement of requiring a { on the ifs is known to
> have positive impact; it's why Rust and Go both require it to my
> understanding.
How is this actually "known"? Does that take the (relatively recent)
-Wmisleading-indentation into account? I think after all these years
where this has not (to my knowledge) caused any problems to coreboot,
"fixing" it now that we even have a compiler check to catch it seems
weird. It's complicating things for humans in favor of something that
a machine can (and already does) solve just as well.
> As for "the kernel" and its coding style, we are going to increasingly
> see people coming from worlds where "the kernel" and its coding style
> matter less and less. Maybe adherence to "the kernel" coding style
> mattered ten years ago; I don't think it is as important now. In my
> case, increasingly, "the kernel" coding style is looking a bit dated.
Are we? Why? I've helped dozens of SoC vendor engineers get started
with coreboot in the last couple of years and they all came from
kernel-style backgrounds (Linux, U-Boot or other projects using the
same style). Of course there are also people coming from elsewhere,
but I don't see kernel style becoming "unimportant" anytime soon. I'd
bet 95+% of coreboot systems run Linux afterwards, and when adding
support for new platforms it's just normal that the hardware vendor
has the same people working on both kernel and firmware drivers for
the same component.
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: More coding style change proposals
If we're going to talk about wasted lines consider this:
we mandate comments as follows:
/*
* something
*/
I just did a test and we have something like 40K lines of white space
spent on that dangling */.
We've got something like 30K if not followed by {
So, were we to to get away from the comments with "wings" (as some
kernel people once called them) we could save all those lines
with just a /* or */ on them. This would more than make up for
additional white lines added by a dangling {.
I note that the changing our comment style would have zero impact on
code safety. The improvement of requiring a { on the ifs is known to
have positive impact; it's why Rust and Go both require it to my
understanding.
As for "the kernel" and its coding style, we are going to increasingly
see people coming from worlds where "the kernel" and its coding style
matter less and less. Maybe adherence to "the kernel" coding style
mattered ten years ago; I don't think it is as important now. In my
case, increasingly, "the kernel" coding style is looking a bit dated.
On Mon, Jun 24, 2019 at 8:14 PM Julius Werner wrote:
>
> Doesn't -Wmisleading-indentation already catch all of this? That's
> enabled by default on the coreboot gcc. I don't think "it's just a
> heuristic" should be a concern unless anyone knows of a real example
> that is otherwise valid coreboot code style but not caught by this
> heuristic. (If we're worried about examples that are not valid code
> style, then changing the code style to make them even more forbidden
> doesn't help... so I think weird cases that mix tab and space
> indentation or the like should count in favor of this.)
>
> If we're concerned that clang-format might cement errors automatically
> then that's a reason for not using clang-format that way, but I don't
> see how changing the coding style would solve it. clang-format's whole
> job is to take whatever input and transform it into the coding style,
> so the input is likely not style-compliant yet.
>
> Forcing braces on single-line statements adds an extra line of
> whitespace where it would otherwise not necessarily belong, which
> hurts readability. How much of a function can fit on a single screen
> is a real readability concern, and I think this style change would
> harm it. That's the same reason we write
>
> while {
>
> instead of
>
> while
> {
>
> like some other projects. (Of course blank lines can also help
> readability, but only in the *right* places and not randomly injected
> by style rules.) It would also move us yet again further away from
> kernel style, causing more issues for people coming from other
> projects.
>
> On Thu, Jun 20, 2019 at 2:54 PM ron minnich wrote:
> >
> > On Thu, Jun 20, 2019 at 1:12 PM Stefan Reinauer
> > wrote:
> > >
> > >
> > >
> > > On 20 Jun 2019 08:26, ron minnich wrote:
> > >
> > > clang-format is not a textual preprocessor. It is basically the ast
> > > builder of followed by output.
> > >
> > > So in your case, I just tried it
> > > main() {
> > >
> > > if (foo)
> > > bar();
> > > baz();
> > > }
> > >
> > > and got the right thing, i.e. braces around bar but not baz.
> > >
> > >
> > > The right thing (e.g. the obviously intended thing) would be too have
> > > braces around both.
> > >
> > > clang-format in this case masks the problem and makes it harder to
> > > identify by expanding the syntax of the unwanted behavior to look
> > > intentional.
> >
> > Nico and Stefan, you make a good point, but I would then argue for
> > even better tools, like clang-tidy:
> > /tmp/x.c:13:10: warning: statement should be inside braces
> > [readability-braces-around-statements]
> > if (foo)
> > ^
> > {
> >
> > In this case, there is a warning thrown, and the author has to clean it up.
> >
> > I don't believe, based on a lot of the history of this sort of problem
> > in C, that we should depend on human reviewers to catch mistakes like
> > this. These tools exist because of a demonstrated need. I think
> > coreboot could benefit from their proper application.
> >
> > You've presented a good case, but what about something like this:
> > if (foo)
> > bar();
> > baz();
> >
> > what's intended? There's an indent, but it's partial. I would not want
> > to guess. But I agree with you that an invisible fixup would be
> > inappropriate.
> > ___
> > coreboot mailing list -- coreboot@coreboot.org
> > To unsubscribe send an email to coreboot-le...@coreboot.org
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: More coding style change proposals
Doesn't -Wmisleading-indentation already catch all of this? That's
enabled by default on the coreboot gcc. I don't think "it's just a
heuristic" should be a concern unless anyone knows of a real example
that is otherwise valid coreboot code style but not caught by this
heuristic. (If we're worried about examples that are not valid code
style, then changing the code style to make them even more forbidden
doesn't help... so I think weird cases that mix tab and space
indentation or the like should count in favor of this.)
If we're concerned that clang-format might cement errors automatically
then that's a reason for not using clang-format that way, but I don't
see how changing the coding style would solve it. clang-format's whole
job is to take whatever input and transform it into the coding style,
so the input is likely not style-compliant yet.
Forcing braces on single-line statements adds an extra line of
whitespace where it would otherwise not necessarily belong, which
hurts readability. How much of a function can fit on a single screen
is a real readability concern, and I think this style change would
harm it. That's the same reason we write
while {
instead of
while
{
like some other projects. (Of course blank lines can also help
readability, but only in the *right* places and not randomly injected
by style rules.) It would also move us yet again further away from
kernel style, causing more issues for people coming from other
projects.
On Thu, Jun 20, 2019 at 2:54 PM ron minnich wrote:
>
> On Thu, Jun 20, 2019 at 1:12 PM Stefan Reinauer
> wrote:
> >
> >
> >
> > On 20 Jun 2019 08:26, ron minnich wrote:
> >
> > clang-format is not a textual preprocessor. It is basically the ast
> > builder of followed by output.
> >
> > So in your case, I just tried it
> > main() {
> >
> > if (foo)
> > bar();
> > baz();
> > }
> >
> > and got the right thing, i.e. braces around bar but not baz.
> >
> >
> > The right thing (e.g. the obviously intended thing) would be too have
> > braces around both.
> >
> > clang-format in this case masks the problem and makes it harder to identify
> > by expanding the syntax of the unwanted behavior to look intentional.
>
> Nico and Stefan, you make a good point, but I would then argue for
> even better tools, like clang-tidy:
> /tmp/x.c:13:10: warning: statement should be inside braces
> [readability-braces-around-statements]
> if (foo)
> ^
> {
>
> In this case, there is a warning thrown, and the author has to clean it up.
>
> I don't believe, based on a lot of the history of this sort of problem
> in C, that we should depend on human reviewers to catch mistakes like
> this. These tools exist because of a demonstrated need. I think
> coreboot could benefit from their proper application.
>
> You've presented a good case, but what about something like this:
> if (foo)
> bar();
> baz();
>
> what's intended? There's an indent, but it's partial. I would not want
> to guess. But I agree with you that an invisible fixup would be
> inappropriate.
> ___
> coreboot mailing list -- coreboot@coreboot.org
> To unsubscribe send an email to coreboot-le...@coreboot.org
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: More coding style change proposals
On Thu, Jun 20, 2019 at 1:12 PM Stefan Reinauer
wrote:
>
>
>
> On 20 Jun 2019 08:26, ron minnich wrote:
>
> clang-format is not a textual preprocessor. It is basically the ast
> builder of followed by output.
>
> So in your case, I just tried it
> main() {
>
> if (foo)
> bar();
> baz();
> }
>
> and got the right thing, i.e. braces around bar but not baz.
>
>
> The right thing (e.g. the obviously intended thing) would be too have braces
> around both.
>
> clang-format in this case masks the problem and makes it harder to identify
> by expanding the syntax of the unwanted behavior to look intentional.
Nico and Stefan, you make a good point, but I would then argue for
even better tools, like clang-tidy:
/tmp/x.c:13:10: warning: statement should be inside braces
[readability-braces-around-statements]
if (foo)
^
{
In this case, there is a warning thrown, and the author has to clean it up.
I don't believe, based on a lot of the history of this sort of problem
in C, that we should depend on human reviewers to catch mistakes like
this. These tools exist because of a demonstrated need. I think
coreboot could benefit from their proper application.
You've presented a good case, but what about something like this:
if (foo)
bar();
baz();
what's intended? There's an indent, but it's partial. I would not want
to guess. But I agree with you that an invisible fixup would be
inappropriate.
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: More coding style change proposals
Am Do., 20. Juni 2019 um 21:47 Uhr schrieb Nico Huber : > That's all true, but it forces an order of the tools you run. e.g. > don't integrate clang-format into your editor because it might hide > the problem before the compiler can warn you. > So don't integrate clang-format into your editor unless you also add some C checker that knows about indentation (eg. a gcc/clang based LSP like https://clang.llvm.org/extra/clangd/) for live feedback. Patrick -- Google Germany GmbH, ABC-Str. 19, 20354 Hamburg Registergericht und -nummer: Hamburg, HRB 86891, Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado ___ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: More coding style change proposals
On 20 Jun 2019 08:26, ron minnich wrote:clang-format is not a textual preprocessor. It is basically the ast
builder of followed by output.
So in your case, I just tried it
main() {
if (foo)
bar();
baz();
}
and got the right thing, i.e. braces around bar but not baz.
The right thing (e.g. the obviously intended thing) would be too have braces around both. clang-format in this case masks the problem and makes it harder to identify by expanding the syntax of the unwanted behavior to look intentional.Stefan
The history of reviewers looking at code is they miss this kind of
error. Constantly. I'm in favor of as much automation as we can get.
ron
On Thu, Jun 20, 2019 at 5:25 AM Nico Huber wrote:
>
> On 20.06.19 06:01, Jacob Garber wrote:
> > On Wed, Jun 19, 2019 at 08:38:14PM -0700, ron minnich wrote:
> >> Given the number of serious problems that lack of braces causes, I
> >> like this proposal. It's indicative that both Rust and Go require the
> >> {}, for reasons of safety.
> >
> > There was a famous vulnerability in Apple's SSL code several years ago
> > because of lack of braces. clang-format can also reformat old code to have
> > mandatory braces if I'm not mistaken.
>
> What will clang-format do if it encounters?
>
> if (foo)
> bar();
> baz();
>
> a)
> if (foo) {
> bar();
> }
> baz();
>
> or b)
> if (foo) {
> bar();
> baz();
> }
>
> Will it spit out a warning? If not, this shows how dangerous automatic
> formatting can be. Because after the formatter run, it's much less ob-
> vious for the reviewer that something is wrong.
>
> Nico
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: More coding style change proposals
On 20.06.19 17:26, ron minnich wrote:
> clang-format is not a textual preprocessor. It is basically the ast
> builder of followed by output.
>
> So in your case, I just tried it
> main() {
>
> if (foo)
> bar();
> baz();
> }
>
> and got the right thing, i.e. braces around bar but not baz.
>
> The history of reviewers looking at code is they miss this kind of
> error. Constantly. I'm in favor of as much automation as we can get.
Sorry, I guess I wasn't clear enough, we're talking past each other.
What I meant is that if somebody writes
if (foo)
bar();
baz();
they most probably forgot the _necessary_ braces and clang-format
would hide this intention even from a human reviewer.
Nico
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: More coding style change proposals
On 20.06.19 18:12, Jacob Garber wrote: > What Ron said, plus if I recall Coverity has a lint for misleading > indentation, so > I don't think there are any current instances of this in the code base. In > fact, > GCC even has -Wmisleading-indentation that was added to -Wall in GCC 6, so we > should currently be ok. That being said, these warnings are only heuristics > (there's > no precise definition of what "misleading" is supposed to be), so I don't > think > they should be relied upon over using braces, which avoids this problem > entirely. That's all true, but it forces an order of the tools you run. e.g. don't integrate clang-format into your editor because it might hide the problem before the compiler can warn you. Nico ___ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: More coding style change proposals
What Ron said, plus if I recall Coverity has a lint for misleading indentation,
so
I don't think there are any current instances of this in the code base. In fact,
GCC even has -Wmisleading-indentation that was added to -Wall in GCC 6, so we
should currently be ok. That being said, these warnings are only heuristics
(there's
no precise definition of what "misleading" is supposed to be), so I don't think
they should be relied upon over using braces, which avoids this problem
entirely.
Jacob
On Thu, Jun 20, 2019 at 08:26:49AM -0700, ron minnich wrote:
> clang-format is not a textual preprocessor. It is basically the ast
> builder of followed by output.
>
> So in your case, I just tried it
> main() {
>
> if (foo)
> bar();
> baz();
> }
>
> and got the right thing, i.e. braces around bar but not baz.
>
> The history of reviewers looking at code is they miss this kind of
> error. Constantly. I'm in favor of as much automation as we can get.
>
> ron
>
> On Thu, Jun 20, 2019 at 5:25 AM Nico Huber wrote:
> >
> > On 20.06.19 06:01, Jacob Garber wrote:
> > > On Wed, Jun 19, 2019 at 08:38:14PM -0700, ron minnich wrote:
> > >> Given the number of serious problems that lack of braces causes, I
> > >> like this proposal. It's indicative that both Rust and Go require the
> > >> {}, for reasons of safety.
> > >
> > > There was a famous vulnerability in Apple's SSL code several years ago
> > > because of lack of braces. clang-format can also reformat old code to have
> > > mandatory braces if I'm not mistaken.
> >
> > What will clang-format do if it encounters?
> >
> > if (foo)
> > bar();
> > baz();
> >
> > a)
> > if (foo) {
> > bar();
> > }
> > baz();
> >
> > or b)
> > if (foo) {
> > bar();
> > baz();
> > }
> >
> > Will it spit out a warning? If not, this shows how dangerous automatic
> > formatting can be. Because after the formatter run, it's much less ob-
> > vious for the reviewer that something is wrong.
> >
> > Nico
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: More coding style change proposals
clang-format is not a textual preprocessor. It is basically the ast
builder of followed by output.
So in your case, I just tried it
main() {
if (foo)
bar();
baz();
}
and got the right thing, i.e. braces around bar but not baz.
The history of reviewers looking at code is they miss this kind of
error. Constantly. I'm in favor of as much automation as we can get.
ron
On Thu, Jun 20, 2019 at 5:25 AM Nico Huber wrote:
>
> On 20.06.19 06:01, Jacob Garber wrote:
> > On Wed, Jun 19, 2019 at 08:38:14PM -0700, ron minnich wrote:
> >> Given the number of serious problems that lack of braces causes, I
> >> like this proposal. It's indicative that both Rust and Go require the
> >> {}, for reasons of safety.
> >
> > There was a famous vulnerability in Apple's SSL code several years ago
> > because of lack of braces. clang-format can also reformat old code to have
> > mandatory braces if I'm not mistaken.
>
> What will clang-format do if it encounters?
>
> if (foo)
> bar();
> baz();
>
> a)
> if (foo) {
> bar();
> }
> baz();
>
> or b)
> if (foo) {
> bar();
> baz();
> }
>
> Will it spit out a warning? If not, this shows how dangerous automatic
> formatting can be. Because after the formatter run, it's much less ob-
> vious for the reviewer that something is wrong.
>
> Nico
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: More coding style change proposals
On 20.06.19 06:01, Jacob Garber wrote:
> On Wed, Jun 19, 2019 at 08:38:14PM -0700, ron minnich wrote:
>> Given the number of serious problems that lack of braces causes, I
>> like this proposal. It's indicative that both Rust and Go require the
>> {}, for reasons of safety.
>
> There was a famous vulnerability in Apple's SSL code several years ago
> because of lack of braces. clang-format can also reformat old code to have
> mandatory braces if I'm not mistaken.
What will clang-format do if it encounters?
if (foo)
bar();
baz();
a)
if (foo) {
bar();
}
baz();
or b)
if (foo) {
bar();
baz();
}
Will it spit out a warning? If not, this shows how dangerous automatic
formatting can be. Because after the formatter run, it's much less ob-
vious for the reviewer that something is wrong.
Nico
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: More coding style change proposals
On Wed, Jun 19, 2019 at 08:38:14PM -0700, ron minnich wrote:
> Given the number of serious problems that lack of braces causes, I
> like this proposal. It's indicative that both Rust and Go require the
> {}, for reasons of safety.
There was a famous vulnerability in Apple's SSL code several years ago
because of lack of braces. clang-format can also reformat old code to have
mandatory braces if I'm not mistaken.
Jacob Garber
https://www.imperialviolet.org/2014/02/22/applebug.html
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: More coding style change proposals
Given the number of serious problems that lack of braces causes, I
like this proposal. It's indicative that both Rust and Go require the
{}, for reasons of safety.
On Wed, Jun 19, 2019 at 11:27 AM Jonathan Neuschäfer
wrote:
>
> On Wed, Jun 19, 2019 at 01:39:50PM -0400, Patrick Georgi via coreboot wrote:
> > Hey everybody,
> >
> > in today's leadership meeting, the question was brought up if we want
> > to normalize the coding style in coreboot to _always_ use braces in
> > if, else and for statements, even if it's just one statement they're
> > wrapping.
> >
> > The arguments made in favor were:
> >
> > 1. it's more consistent
> > 2. it's safer: you won't accidentally add a statement that is outside
> >the context it's supposed to run in (or even move a statement out
> >into the parent context by inserting another statement)
> >
> > So instead of:
> >
> > if (foo)
> > bar();
> > else {
> > baz();
> > quux();
> > }
> >
> > we'd do:
> >
> > if (foo) {
> > bar();
> > } else {
> > baz();
> > quux();
> > }
>
> Quick sidenote: Under the old rules[1], inherited from the Linux
> kernel coding style[2], the above example would look the same:
>
> | [...]
> | This does not apply if only one branch of a conditional statement is a
> | single statement; in the latter case use braces in both branches:
> |
> | if (condition) {
> | do_this();
> | do_that();
> | } else {
> | otherwise();
> | }
>
> ... so a better example would be one where technically no parentheses
> are required at all:
>
> if (foo)
> bar();
> else
> baz();
>
> becomes:
>
> if (foo) {
> bar();
> } else {
> baz();
> }
>
>
> greetings,
> Jonathan Neuschäfer
>
>
> [1]: https://doc.coreboot.org/coding_style.html#placing-braces-and-spaces
> [2]:
> https://www.kernel.org/doc/html/latest/process/coding-style.html?highlight=coding%20style#placing-braces-and-spaces
> ___
> coreboot mailing list -- coreboot@coreboot.org
> To unsubscribe send an email to coreboot-le...@coreboot.org
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: More coding style change proposals
On Wed, Jun 19, 2019 at 01:39:50PM -0400, Patrick Georgi via coreboot wrote:
> Hey everybody,
>
> in today's leadership meeting, the question was brought up if we want
> to normalize the coding style in coreboot to _always_ use braces in
> if, else and for statements, even if it's just one statement they're
> wrapping.
>
> The arguments made in favor were:
>
> 1. it's more consistent
> 2. it's safer: you won't accidentally add a statement that is outside
>the context it's supposed to run in (or even move a statement out
>into the parent context by inserting another statement)
>
> So instead of:
>
> if (foo)
> bar();
> else {
> baz();
> quux();
> }
>
> we'd do:
>
> if (foo) {
> bar();
> } else {
> baz();
> quux();
> }
Quick sidenote: Under the old rules[1], inherited from the Linux
kernel coding style[2], the above example would look the same:
| [...]
| This does not apply if only one branch of a conditional statement is a
| single statement; in the latter case use braces in both branches:
|
| if (condition) {
| do_this();
| do_that();
| } else {
| otherwise();
| }
... so a better example would be one where technically no parentheses
are required at all:
if (foo)
bar();
else
baz();
becomes:
if (foo) {
bar();
} else {
baz();
}
greetings,
Jonathan Neuschäfer
[1]: https://doc.coreboot.org/coding_style.html#placing-braces-and-spaces
[2]:
https://www.kernel.org/doc/html/latest/process/coding-style.html?highlight=coding%20style#placing-braces-and-spaces
signature.asc
Description: PGP signature
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
