* [U-Boot] Question about Coding-Style
@ 2014-02-04 12:50 Hannes Petermaier
2014-02-04 14:14 ` Stefano Babic
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Hannes Petermaier @ 2014-02-04 12:50 UTC (permalink / raw)
To: u-boot
Hello,
I've read on the U-Boot website about coding style. They say 'All
contributions to U-Boot should conform to the Linux kernel coding style'.
Further i've read this link and there are at least 2 things which i have
troubles with.
1)
a tab-ident is 8 spaces (in eclipse for example with the built in K&R has
4 characters). With 8 characters per TAB the problem from point 2 enters
very, very fast.
2)
a line ist limited to 80 characters (maybe thus comes from very old days
where displays couldn't show more than this and scrolling was very
expensive). is this state of the art ?
In fact i've found a lot of files within the u-boot code which do not obey
to this rules.
Now my question is, how strong are this two points ?
checkpatch.pl rails against a patch file with a lot of warnings if there
is a line longer than 80bytes, also it takes TABS as 8 spaces.
whats your opinion about this ?
Last question:
Is this mailing list the right place of discussing such things ?
many thanks and best regards,
Hannes
mfG
Petermaier Hannes
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] Question about Coding-Style
2014-02-04 12:50 [U-Boot] Question about Coding-Style Hannes Petermaier
@ 2014-02-04 14:14 ` Stefano Babic
2014-02-04 14:18 ` Michael Trimarchi
2014-02-04 14:50 ` Hannes Petermaier
2014-02-04 15:02 ` Tom Rini
` (2 subsequent siblings)
3 siblings, 2 replies; 15+ messages in thread
From: Stefano Babic @ 2014-02-04 14:14 UTC (permalink / raw)
To: u-boot
Hi Hannes,
On 04/02/2014 13:50, Hannes Petermaier wrote:
> Hello,
>
> I've read on the U-Boot website about coding style. They say 'All
> contributions to U-Boot should conform to the Linux kernel coding style'.
> Further i've read this link and there are at least 2 things which i have
> troubles with.
>
> 1)
> a tab-ident is 8 spaces
Right
> (in eclipse for example with the built in K&R has
> 4 characters). With 8 characters per TAB the problem from point 2 enters
> very, very fast.
>
> 2)
> a line ist limited to 80 characters (maybe thus comes from very old days
> where displays couldn't show more than this and scrolling was very
> expensive). is this state of the art ?
Yes, it is. Same rule as in kernel.
>
> In fact i've found a lot of files within the u-boot code which do not obey
> to this rules.
I cannot tell for each exceptions you found. Maybe the code is very old,
and it slipped into mainline without fixing the lenght. There are also
some well-known exception (generally for tables), where a longer line
was accepted after discussion in the ML to increase readability.
(tables for pinmux, as example arch/arm/include/asm/arch-mx6/mx6dl_pins.h).
>
> Now my question is, how strong are this two points ?
They are strong until there is a general acceptance on the ML to drop
the rule. However, the rule was already discussed in the past (you can
dig deep in archives), and the result was to maintain it, exactly as it
is valid for linux.
> checkpatch.pl rails against a patch file with a lot of warnings if there
> is a line longer than 80bytes, also it takes TABS as 8 spaces.
checkpatch is run before merging a patch into a merge tree by
custodians. If checkpatch reports errors, they must be fixed.
>
> whats your opinion about this ?
>
> Last question:
> Is this mailing list the right place of discussing such things ?
>
I think you are in the right place to discuss these topics ;-)
Best regards,
Stefano Babic
--
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] Question about Coding-Style
2014-02-04 14:14 ` Stefano Babic
@ 2014-02-04 14:18 ` Michael Trimarchi
2014-02-04 14:50 ` Hannes Petermaier
1 sibling, 0 replies; 15+ messages in thread
From: Michael Trimarchi @ 2014-02-04 14:18 UTC (permalink / raw)
To: u-boot
Hi
Il 04/feb/2014 15:15 "Stefano Babic" <sbabic@denx.de> ha scritto:
>
> Hi Hannes,
>
> On 04/02/2014 13:50, Hannes Petermaier wrote:
> > Hello,
> >
> > I've read on the U-Boot website about coding style. They say 'All
> > contributions to U-Boot should conform to the Linux kernel coding
style'.
> > Further i've read this link and there are at least 2 things which i have
> > troubles with.
> >
> > 1)
> > a tab-ident is 8 spaces
>
> Right
>
> > (in eclipse for example with the built in K&R has
> > 4 characters). With 8 characters per TAB the problem from point 2 enters
> > very, very fast.
If it happens your code has two many nested block
Michael
> >
> > 2)
> > a line ist limited to 80 characters (maybe thus comes from very old days
> > where displays couldn't show more than this and scrolling was very
> > expensive). is this state of the art ?
>
> Yes, it is. Same rule as in kernel.
>
> >
> > In fact i've found a lot of files within the u-boot code which do not
obey
> > to this rules.
>
> I cannot tell for each exceptions you found. Maybe the code is very old,
> and it slipped into mainline without fixing the lenght. There are also
> some well-known exception (generally for tables), where a longer line
> was accepted after discussion in the ML to increase readability.
> (tables for pinmux, as example
arch/arm/include/asm/arch-mx6/mx6dl_pins.h).
>
> >
> > Now my question is, how strong are this two points ?
>
> They are strong until there is a general acceptance on the ML to drop
> the rule. However, the rule was already discussed in the past (you can
> dig deep in archives), and the result was to maintain it, exactly as it
> is valid for linux.
>
>
> > checkpatch.pl rails against a patch file with a lot of warnings if there
> > is a line longer than 80bytes, also it takes TABS as 8 spaces.
>
> checkpatch is run before merging a patch into a merge tree by
> custodians. If checkpatch reports errors, they must be fixed.
>
> >
> > whats your opinion about this ?
> >
> > Last question:
> > Is this mailing list the right place of discussing such things ?
> >
>
> I think you are in the right place to discuss these topics ;-)
>
> Best regards,
> Stefano Babic
>
> --
> =====================================================================
> DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
> =====================================================================
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] Question about Coding-Style
2014-02-04 14:14 ` Stefano Babic
2014-02-04 14:18 ` Michael Trimarchi
@ 2014-02-04 14:50 ` Hannes Petermaier
2014-02-04 15:02 ` Stefano Babic
2014-02-04 20:32 ` Wolfgang Denk
1 sibling, 2 replies; 15+ messages in thread
From: Hannes Petermaier @ 2014-02-04 14:50 UTC (permalink / raw)
To: u-boot
Stefano,
many thanks.
This will be a hard challenge for me to obey to the rules :-)
The 80 character thing is reported from checkpatch.pl as WARNING, not as
ERROR - so there will be a chance of the patch to become accepted by a
custodian.
In between i tried to reformat by Code to obey to these rules, but now
checkpatch.pl tells me following:
WARNING: Avoid unnecessary line continuations
#531: FILE: board/BuR/bur_tseries/board.c:155:
+ if (tps65217_voltage_update(TPS65217_DEFDCDC3, \
in real life the code section looks like this:
/* Set DCDC3 (CORE) voltage to 1.125V */
if (tps65217_voltage_update(TPS65217_DEFDCDC3, \
TPS65217_DCDC_VOLT_SEL_1125MV)) {
puts("tps65217_voltage_update failure\n");
return;
}
If i don't do the line break at line 155 checkpatch.pl is disturbing me
with the 80 character rule.
Another thing is linewrapping of output strings, to obey to the rules i
have to format the string as following:
if (i2c_probe(TPS65217_CHIP_PM)) {
printf("PMIC chip (0x%02x) not present! skipping" \
"further configuration.\n", TPS65217_CHIP_PM);
return;
}
But this makes it impossible to grep the code in case of an error.
Any idea how to deal with such things ?
best regards,
Hannes
Stefano Babic wrote:
> Hi Hannes,
>
> On 04/02/2014 13:50, Hannes Petermaier wrote:
>> Hello,
>>
>> I've read on the U-Boot website about coding style. They say 'All
>> contributions to U-Boot should conform to the Linux kernel coding
>> style'.
>> Further i've read this link and there are at least 2 things which i have
>> troubles with.
>>
>> 1)
>> a tab-ident is 8 spaces
>
> Right
>
>> (in eclipse for example with the built in K&R has
>> 4 characters). With 8 characters per TAB the problem from point 2 enters
>> very, very fast.
>>
>> 2)
>> a line ist limited to 80 characters (maybe thus comes from very old days
>> where displays couldn't show more than this and scrolling was very
>> expensive). is this state of the art ?
>
> Yes, it is. Same rule as in kernel.
>
>>
>> In fact i've found a lot of files within the u-boot code which do not
>> obey
>> to this rules.
>
> I cannot tell for each exceptions you found. Maybe the code is very old,
> and it slipped into mainline without fixing the lenght. There are also
> some well-known exception (generally for tables), where a longer line
> was accepted after discussion in the ML to increase readability.
> (tables for pinmux, as example
> arch/arm/include/asm/arch-mx6/mx6dl_pins.h).
>
>>
>> Now my question is, how strong are this two points ?
>
> They are strong until there is a general acceptance on the ML to drop
> the rule. However, the rule was already discussed in the past (you can
> dig deep in archives), and the result was to maintain it, exactly as it
> is valid for linux.
>
>
>> checkpatch.pl rails against a patch file with a lot of warnings if there
>> is a line longer than 80bytes, also it takes TABS as 8 spaces.
>
> checkpatch is run before merging a patch into a merge tree by
> custodians. If checkpatch reports errors, they must be fixed.
>
>>
>> whats your opinion about this ?
>>
>> Last question:
>> Is this mailing list the right place of discussing such things ?
>>
>
> I think you are in the right place to discuss these topics ;-)
>
> Best regards,
> Stefano Babic
>
> --
> =====================================================================
> DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
> =====================================================================
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] Question about Coding-Style
2014-02-04 12:50 [U-Boot] Question about Coding-Style Hannes Petermaier
2014-02-04 14:14 ` Stefano Babic
@ 2014-02-04 15:02 ` Tom Rini
2014-02-04 20:20 ` Wolfgang Denk
2014-02-04 22:03 ` Scott Wood
3 siblings, 0 replies; 15+ messages in thread
From: Tom Rini @ 2014-02-04 15:02 UTC (permalink / raw)
To: u-boot
On Tue, Feb 04, 2014 at 12:50:18PM -0000, Hannes Petermaier wrote:
> Hello,
>
> I've read on the U-Boot website about coding style. They say 'All
> contributions to U-Boot should conform to the Linux kernel coding style'.
> Further i've read this link and there are at least 2 things which i have
> troubles with.
>
> 1)
> a tab-ident is 8 spaces (in eclipse for example with the built in K&R has
> 4 characters). With 8 characters per TAB the problem from point 2 enters
> very, very fast.
>
> 2)
> a line ist limited to 80 characters (maybe thus comes from very old days
> where displays couldn't show more than this and scrolling was very
> expensive). is this state of the art ?
>
> In fact i've found a lot of files within the u-boot code which do not obey
> to this rules.
The exceptions to the line length rule are printed strings, as breaking
them up can make debugging harder than it should be.
> Now my question is, how strong are this two points ?
> checkpatch.pl rails against a patch file with a lot of warnings if there
> is a line longer than 80bytes, also it takes TABS as 8 spaces.
>
> whats your opinion about this ?
>
> Last question:
> Is this mailing list the right place of discussing such things ?
It's not debatable, really. It's important for a project to be
consistent (as much as possible) in coding style, and if it helps,
recall the old adage about a good compromise leaves everyone unhappy
about something.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140204/0729abd8/attachment.pgp>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] Question about Coding-Style
2014-02-04 14:50 ` Hannes Petermaier
@ 2014-02-04 15:02 ` Stefano Babic
2014-02-04 15:07 ` Tom Rini
2014-02-04 20:32 ` Wolfgang Denk
1 sibling, 1 reply; 15+ messages in thread
From: Stefano Babic @ 2014-02-04 15:02 UTC (permalink / raw)
To: u-boot
Hi Hannes,
On 04/02/2014 15:50, Hannes Petermaier wrote:
> The 80 character thing is reported from checkpatch.pl as WARNING, not as
> ERROR - so there will be a chance of the patch to become accepted by a
> custodian.
No, it will *not* be accepted. There must be a very good reason to
accept it.
As far as I know, there are only a few warnings that are generally
accepted (I think using typedf is one of them).
> In between i tried to reformat by Code to obey to these rules, but now
> checkpatch.pl tells me following:
>
> WARNING: Avoid unnecessary line continuations
> #531: FILE: board/BuR/bur_tseries/board.c:155:
> + if (tps65217_voltage_update(TPS65217_DEFDCDC3, \
Why do you need "\" ? It is not a macro, checkpatch is right.
>
> in real life the code section looks like this:
> /* Set DCDC3 (CORE) voltage to 1.125V */
> if (tps65217_voltage_update(TPS65217_DEFDCDC3, \
> TPS65217_DCDC_VOLT_SEL_1125MV)) {
Again, you do not need the line continuation because it is not a macro.
The compiler knows that the statement is not yet finished.
> puts("tps65217_voltage_update failure\n");
> return;
> }
>
> If i don't do the line break at line 155 checkpatch.pl is disturbing me
> with the 80 character rule.
Simply drop the unnecessary "\".
>
> Another thing is linewrapping of output strings, to obey to the rules i
> have to format the string as following:
>
> if (i2c_probe(TPS65217_CHIP_PM)) {
> printf("PMIC chip (0x%02x) not present! skipping" \
> "further configuration.\n", TPS65217_CHIP_PM);
> return;
> }
>
> But this makes it impossible to grep the code in case of an error.
You must combine a more complicate grep, maybe with the -A (after
context) option or using a regexp. However, this is not a reason to
break the rule.
Best regards,
Stefano Babic
--
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] Question about Coding-Style
2014-02-04 15:02 ` Stefano Babic
@ 2014-02-04 15:07 ` Tom Rini
2014-02-10 9:58 ` Albert ARIBAUD
0 siblings, 1 reply; 15+ messages in thread
From: Tom Rini @ 2014-02-04 15:07 UTC (permalink / raw)
To: u-boot
On Tue, Feb 04, 2014 at 04:02:56PM +0100, Stefano Babic wrote:
> Hi Hannes,
>
> On 04/02/2014 15:50, Hannes Petermaier wrote:
[snip]
> > Another thing is linewrapping of output strings, to obey to the rules i
> > have to format the string as following:
> >
> > if (i2c_probe(TPS65217_CHIP_PM)) {
> > printf("PMIC chip (0x%02x) not present! skipping" \
> > "further configuration.\n", TPS65217_CHIP_PM);
> > return;
> > }
> >
> > But this makes it impossible to grep the code in case of an error.
>
> You must combine a more complicate grep, maybe with the -A (after
> context) option or using a regexp. However, this is not a reason to
> break the rule.
Strings are the reason to break the rule and we have checkpatch patched
(mostly?) to not complain. It's even true in the kernel.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140204/95eb70c7/attachment.pgp>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] Question about Coding-Style
2014-02-04 12:50 [U-Boot] Question about Coding-Style Hannes Petermaier
2014-02-04 14:14 ` Stefano Babic
2014-02-04 15:02 ` Tom Rini
@ 2014-02-04 20:20 ` Wolfgang Denk
2014-02-04 22:03 ` Scott Wood
3 siblings, 0 replies; 15+ messages in thread
From: Wolfgang Denk @ 2014-02-04 20:20 UTC (permalink / raw)
To: u-boot
Dear Hannes,
In message <11685.213.33.116.116.1391518218.squirrel@petermaier.org> you wrote:
>
> I've read on the U-Boot website about coding style. They say 'All
> contributions to U-Boot should conform to the Linux kernel coding style'.
> Further i've read this link and there are at least 2 things which i have
> troubles with.
>
> 1)
> a tab-ident is 8 spaces (in eclipse for example with the built in K&R has
> 4 characters). With 8 characters per TAB the problem from point 2 enters
> very, very fast.
Please read the CodingStyle document completely. It says:
... and have the added benefit of warning you when you're
nesting your functions too deep. Heed that warning.
> 2)
> a line ist limited to 80 characters (maybe thus comes from very old days
> where displays couldn't show more than this and scrolling was very
> expensive). is this state of the art ?
Yes, it is. Please read the CodingStyle document.
> In fact i've found a lot of files within the u-boot code which do not obey
> to this rules.
That should not be that many files, resp. only such where there were
good reasons for allowing for longer lines (like tables of GPIO
initializations, not breaking printable strings, etc.). Of course,
poorly formatted code can be found here and there - cleanup patches
are always welcome.
> Now my question is, how strong are this two points ?
> checkpatch.pl rails against a patch file with a lot of warnings if there
> is a line longer than 80bytes, also it takes TABS as 8 spaces.
>
> whats your opinion about this ?
Please fix these issues before posting the patch.
> Is this mailing list the right place of discussing such things ?
Sure.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The only time the world beats a path to your door is when you are in
the bathroom.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] Question about Coding-Style
2014-02-04 14:50 ` Hannes Petermaier
2014-02-04 15:02 ` Stefano Babic
@ 2014-02-04 20:32 ` Wolfgang Denk
1 sibling, 0 replies; 15+ messages in thread
From: Wolfgang Denk @ 2014-02-04 20:32 UTC (permalink / raw)
To: u-boot
Dear Hannes,
In message <29703.213.33.116.113.1391525447.squirrel@petermaier.org> you wrote:
>
> The 80 character thing is reported from checkpatch.pl as WARNING, not as
> ERROR - so there will be a chance of the patch to become accepted by a
> custodian.
It is better for your reputation ere on the list not to try to sneak
poorly formatted code in. Just play by the rules, and these include
to fix such warnings issues by checkpatch.
> In between i tried to reformat by Code to obey to these rules, but now
> checkpatch.pl tells me following:
>
> WARNING: Avoid unnecessary line continuations
> #531: FILE: board/BuR/bur_tseries/board.c:155:
> + if (tps65217_voltage_update(TPS65217_DEFDCDC3, \
>
> in real life the code section looks like this:
> /* Set DCDC3 (CORE) voltage to 1.125V */
> if (tps65217_voltage_update(TPS65217_DEFDCDC3, \
> TPS65217_DCDC_VOLT_SEL_1125MV)) {
> puts("tps65217_voltage_update failure\n");
> return;
> }
>
> If i don't do the line break at line 155 checkpatch.pl is disturbing me
> with the 80 character rule.
You can of course break the lines, but why do you put a backslash at
the end of the line? It is this what checkpatch is complaining about:
"unnecessary line continuations".
> Another thing is linewrapping of output strings, to obey to the rules i
> have to format the string as following:
>
> if (i2c_probe(TPS65217_CHIP_PM)) {
> printf("PMIC chip (0x%02x) not present! skipping" \
> "further configuration.\n", TPS65217_CHIP_PM);
> return;
> }
No. printable strings shall not be broken, as you ay want to grep the
source code for that very string you see in your console log.
But then, it might make sense to think if you can cose a shorter form
of the text? In the above case, the line is > 60 characters long
(BTW: you missed a space when breaking the string). Is this needed?
For example:
printf("ERROR: PMIC at 0x%02x missing\n", TPS65217_CHIP_PM);
gives the same information, actually even more prominent in the boot
log - and I don;t even have top break the line.
> But this makes it impossible to grep the code in case of an error.
>
> Any idea how to deal with such things ?
Be creative, and use common sense.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
C++ was an interesting and valuable experiment, but we've learned its
lessons and it's time to move on.
- Peter Curran in <DCqM4z.BxB@isgtec.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] Question about Coding-Style
2014-02-04 12:50 [U-Boot] Question about Coding-Style Hannes Petermaier
` (2 preceding siblings ...)
2014-02-04 20:20 ` Wolfgang Denk
@ 2014-02-04 22:03 ` Scott Wood
2014-02-12 13:21 ` Fabio Estevam
3 siblings, 1 reply; 15+ messages in thread
From: Scott Wood @ 2014-02-04 22:03 UTC (permalink / raw)
To: u-boot
On Tue, 2014-02-04 at 12:50 +0000, Hannes Petermaier wrote:
> 2)
> a line ist limited to 80 characters (maybe thus comes from very old days
> where displays couldn't show more than this and scrolling was very
> expensive). is this state of the art ?
Even on a fairly nice, modern screen, I find 80 columns to be about
right for two side-by-side windows with a comfortable font size.
Plus, horizontal scrolling is still a pain[1], and excessively long
lines can be harder to read even if they fit on the screen (this is why
newspapers and other documents often format text in columns).
-Scott
[1] I'm looking at you, new boards.cfg format!
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] Question about Coding-Style
2014-02-04 15:07 ` Tom Rini
@ 2014-02-10 9:58 ` Albert ARIBAUD
2014-02-10 10:16 ` Chris Moore
0 siblings, 1 reply; 15+ messages in thread
From: Albert ARIBAUD @ 2014-02-10 9:58 UTC (permalink / raw)
To: u-boot
Hi Tom,
On Tue, 4 Feb 2014 10:07:32 -0500, Tom Rini <trini@ti.com> wrote:
> On Tue, Feb 04, 2014 at 04:02:56PM +0100, Stefano Babic wrote:
> > Hi Hannes,
> >
> > On 04/02/2014 15:50, Hannes Petermaier wrote:
> [snip]
> > > Another thing is linewrapping of output strings, to obey to the rules i
> > > have to format the string as following:
> > >
> > > if (i2c_probe(TPS65217_CHIP_PM)) {
> > > printf("PMIC chip (0x%02x) not present! skipping" \
> > > "further configuration.\n", TPS65217_CHIP_PM);
> > > return;
> > > }
> > >
> > > But this makes it impossible to grep the code in case of an error.
> >
> > You must combine a more complicate grep, maybe with the -A (after
> > context) option or using a regexp. However, this is not a reason to
> > break the rule.
>
> Strings are the reason to break the rule and we have checkpatch patched
> (mostly?) to not complain. It's even true in the kernel.
Hmm... Last time I checked,
"abc"
"def"
Is a valid C string, and does not require a backslash. Do I miss
something?
Amicalement,
--
Albert.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] Question about Coding-Style
2014-02-10 9:58 ` Albert ARIBAUD
@ 2014-02-10 10:16 ` Chris Moore
2014-02-10 10:54 ` Albert ARIBAUD
2014-02-10 18:27 ` Wolfgang Denk
0 siblings, 2 replies; 15+ messages in thread
From: Chris Moore @ 2014-02-10 10:16 UTC (permalink / raw)
To: u-boot
Hi Albert,
Le 10/02/2014 10:58, Albert ARIBAUD a ?crit :
> Hi Tom,
>
> On Tue, 4 Feb 2014 10:07:32 -0500, Tom Rini<trini@ti.com> wrote:
>
>> On Tue, Feb 04, 2014 at 04:02:56PM +0100, Stefano Babic wrote:
>>> Hi Hannes,
>>>
>>> On 04/02/2014 15:50, Hannes Petermaier wrote:
>> [snip]
>>>> Another thing is linewrapping of output strings, to obey to the rules i
>>>> have to format the string as following:
>>>>
>>>> if (i2c_probe(TPS65217_CHIP_PM)) {
>>>> printf("PMIC chip (0x%02x) not present! skipping" \
>>>> "further configuration.\n", TPS65217_CHIP_PM);
>>>> return;
>>>> }
>>>>
>>>> But this makes it impossible to grep the code in case of an error.
>>> You must combine a more complicate grep, maybe with the -A (after
>>> context) option or using a regexp. However, this is not a reason to
>>> break the rule.
>> Strings are the reason to break the rule and we have checkpatch patched
>> (mostly?) to not complain. It's even true in the kernel.
> Hmm... Last time I checked,
>
> "abc"
> "def"
>
> Is a valid C string, and does not require a backslash. Do I miss
> something?
Yes, you are of course correct: the result is the C string "abcdef" and
the backslash in the original is superfluous.
However a grep for "abcdef" in the source code won't match it :(
This is why splitting strings like this is discouraged in the Linux kernel.
Cheers,
Chris
---
Ce courrier ?lectronique ne contient aucun virus ou logiciel malveillant parce que la protection avast! Antivirus est active.
http://www.avast.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] Question about Coding-Style
2014-02-10 10:16 ` Chris Moore
@ 2014-02-10 10:54 ` Albert ARIBAUD
2014-02-10 18:27 ` Wolfgang Denk
1 sibling, 0 replies; 15+ messages in thread
From: Albert ARIBAUD @ 2014-02-10 10:54 UTC (permalink / raw)
To: u-boot
Hi Chris,
> > Hmm... Last time I checked,
> >
> > "abc"
> > "def"
> >
> > Is a valid C string, and does not require a backslash. Do I miss
> > something?
>
> Yes, you are of course correct: the result is the C string "abcdef" and
> the backslash in the original is superfluous.
> However a grep for "abcdef" in the source code won't match it :(
> This is why splitting strings like this is discouraged in the Linux kernel.
Got it, thanks.
> Cheers,
> Chris
Amicalement,
--
Albert.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] Question about Coding-Style
2014-02-10 10:16 ` Chris Moore
2014-02-10 10:54 ` Albert ARIBAUD
@ 2014-02-10 18:27 ` Wolfgang Denk
1 sibling, 0 replies; 15+ messages in thread
From: Wolfgang Denk @ 2014-02-10 18:27 UTC (permalink / raw)
To: u-boot
Dear Chris,
In message <52F8A706.7030103@free.fr> you wrote:
>
> > "abc"
> > "def"
> >
> > Is a valid C string, and does not require a backslash. Do I miss
> > something?
>
> Yes, you are of course correct: the result is the C string "abcdef" and =
>
> the backslash in the original is superfluous.
> However a grep for "abcdef" in the source code won't match it :(
> This is why splitting strings like this is discouraged in the Linux kernel.
To be precise, the Linux CodingStyle refers to "user-visible strings
such as printk messages".
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The software required `Windows 95 or better', so I installed Linux.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] Question about Coding-Style
2014-02-04 22:03 ` Scott Wood
@ 2014-02-12 13:21 ` Fabio Estevam
0 siblings, 0 replies; 15+ messages in thread
From: Fabio Estevam @ 2014-02-12 13:21 UTC (permalink / raw)
To: u-boot
On Tue, Feb 4, 2014 at 8:03 PM, Scott Wood <scottwood@freescale.com> wrote:
> Plus, horizontal scrolling is still a pain[1], and excessively long
> lines can be harder to read even if they fit on the screen (this is why
> newspapers and other documents often format text in columns).
>
> -Scott
>
> [1] I'm looking at you, new boards.cfg format!
I agree with you, Scott :-)
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2014-02-12 13:21 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-04 12:50 [U-Boot] Question about Coding-Style Hannes Petermaier
2014-02-04 14:14 ` Stefano Babic
2014-02-04 14:18 ` Michael Trimarchi
2014-02-04 14:50 ` Hannes Petermaier
2014-02-04 15:02 ` Stefano Babic
2014-02-04 15:07 ` Tom Rini
2014-02-10 9:58 ` Albert ARIBAUD
2014-02-10 10:16 ` Chris Moore
2014-02-10 10:54 ` Albert ARIBAUD
2014-02-10 18:27 ` Wolfgang Denk
2014-02-04 20:32 ` Wolfgang Denk
2014-02-04 15:02 ` Tom Rini
2014-02-04 20:20 ` Wolfgang Denk
2014-02-04 22:03 ` Scott Wood
2014-02-12 13:21 ` Fabio Estevam
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.