Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
Hi David, On Sat, 29 Sep 2007 15:30:15 -0700, David Brownell wrote: > > > > Let's kill it, please. (i.e., ACK) > > > > > > But ... why? What value could needless parens provide? By definition of "needless", none. But the question is precisely whether the parentheses are always needless. The way your formulate your question suggests that you aren't actually interested in discussing the matter, so I am wondering why are you taking part at all. > > Who says that needless parens could provide value? > > Jean, which is why he submitted the patch. No, I didn't. I said that parentheses were sometimes useful. You said that parentheses were usually needless (which isn't that different, BTW.) Combining both statements into "useless parentheses provide value" is a logical flaw. And attributing it to me is dishonest. > (...) > > > I'd kind of think a change like this should have some > > > positive motivation. My "positive motivation" was that this statement, as is, is more confusing than helpful. As a matter of fact, it led us to disagree on a patch you sent. And I would like contributors to focus on the more important parts of CodingStyle. If someone wants to improve the statement instead of removing it, that's fine with me, but not having the time to work on this, I proposed the removal. > I would agree that line could be improved to say that messages > should not be needlessly large. It does already: "Make the messages concise, clear, and unambiguous." -- Jean Delvare - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
Hi Andrew,
On Sat, 29 Sep 2007 03:51:56 -0700, Andrew Morton wrote:
> On Sat, 29 Sep 2007 12:25:30 +0200 Jean Delvare <[EMAIL PROTECTED]> wrote:
>
> > Remove a not particularly relevant rule from CodingStyle.
> > Sometimes, printing numbers in parentheses doesn't add value, but in
> > some (most?) cases it makes the message easier to read. As a matter of
> > fact, this practice is widely used in the kernel:
> >
> > linux-2.6.23-rc8$ quilt grep -I '(%l*[du])' | wc -l
> > 3166
> > linux-2.6.23-rc8$
> >
> > Signed-off-by: Jean Delvare <[EMAIL PROTECTED]>
> > ---
> > Documentation/CodingStyle |2 --
> > 1 file changed, 2 deletions(-)
> >
> > --- linux-2.6.23-rc8.orig/Documentation/CodingStyle 2007-07-23
> > 16:44:32.0 +0200
> > +++ linux-2.6.23-rc8/Documentation/CodingStyle 2007-09-28
> > 23:53:23.0 +0200
> > @@ -638,8 +638,6 @@ concise, clear, and unambiguous.
> >
> > Kernel messages do not have to be terminated with a period.
> >
> > -Printing numbers in parentheses (%d) adds no value and should be avoided.
> > -
> > There are a number of driver model diagnostic macros in
> > which you should use to make sure messages are matched to the right device
> > and driver, and are tagged with the right level: dev_err(), dev_warn(),
>
> I wonder how that got there.
>
> Printing something like
>
> bytes remaining: 0x12 (18)
>
> is a quite logical thing to do, although pretty darm pointless.
>
>
> otoh, looking at the various instances, we have lots of stuff like this:
>
>
> printk(KERN_ERR "seq-oss: unable to delete queue %d (%d)\n", queue, rc);
>
> which I would argue is wrong and is inconsistent with most other error
> reporting. It should be
>
> unable to delete queue %d: %d
I disagree. Reporting an error code is precisely the case where I think
(%d) is valuable. A colon suggests that an understandable explanation
follows, while an error code isn't that understandable. What I expect
as a user is:
Read failed: I/O error
but:
Read failed (-5)
And I thought this was the usual convention, too, but you seem to
disagree.
> And this:
>
> printk(KERN_ERR "%s: context size (%u) exceeds payload "
>
> doesn't need the parens
Agreed.
> Here:
>
> printk("hardirqs last enabled at (%u): ", curr->hardirq_enable_event);
> print_ip_sym(curr->hardirq_enable_ip);
> printk("hardirqs last disabled at (%u): ", curr->hardirq_disable_event);
> print_ip_sym(curr->hardirq_disable_ip);
> printk("softirqs last enabled at (%u): ", curr->softirq_enable_event);
> print_ip_sym(curr->softirq_enable_ip);
> printk("softirqs last disabled at (%u): ", curr->softirq_disable_event);
> print_ip_sym(curr->softirq_disable_ip);
>
> all the parens are just illogical and should be removed.
Agreed.
> Here:
>
> xlog_warn("XFS: %s: unrecognised log version (%d).",
> __FUNCTION__, INT_GET(rhead->h_version, ARCH_CONVERT));
>
> should use "unrecognised log version: %d"
I'm fine both ways.
> This:
>
> printk(KERN_ERR "udf: unknown compression code (%d) stri=%s\n",
> cmp_id, ocu_i->u_name);
>
> should use colon as well.
>
>
> So in fact a large number of the instances I see in there are illogical and
> basically gramatically wrong and should be converted to use a colon.
I would be fine having one or more explicit rules in CodingStyle on how
log messages should be formatted, if you think it adds value. But a
general statement "don't do foo" when it's sometimes just fine to do
foo, doesn't add any value IMHO.
--
Jean Delvare
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
Hi Andrew, On Sat, 29 Sep 2007 03:51:56 -0700, Andrew Morton wrote: On Sat, 29 Sep 2007 12:25:30 +0200 Jean Delvare [EMAIL PROTECTED] wrote: Remove a not particularly relevant rule from CodingStyle. Sometimes, printing numbers in parentheses doesn't add value, but in some (most?) cases it makes the message easier to read. As a matter of fact, this practice is widely used in the kernel: linux-2.6.23-rc8$ quilt grep -I '(%l*[du])' | wc -l 3166 linux-2.6.23-rc8$ Signed-off-by: Jean Delvare [EMAIL PROTECTED] --- Documentation/CodingStyle |2 -- 1 file changed, 2 deletions(-) --- linux-2.6.23-rc8.orig/Documentation/CodingStyle 2007-07-23 16:44:32.0 +0200 +++ linux-2.6.23-rc8/Documentation/CodingStyle 2007-09-28 23:53:23.0 +0200 @@ -638,8 +638,6 @@ concise, clear, and unambiguous. Kernel messages do not have to be terminated with a period. -Printing numbers in parentheses (%d) adds no value and should be avoided. - There are a number of driver model diagnostic macros in linux/device.h which you should use to make sure messages are matched to the right device and driver, and are tagged with the right level: dev_err(), dev_warn(), I wonder how that got there. Printing something like bytes remaining: 0x12 (18) is a quite logical thing to do, although pretty darm pointless. otoh, looking at the various instances, we have lots of stuff like this: printk(KERN_ERR seq-oss: unable to delete queue %d (%d)\n, queue, rc); which I would argue is wrong and is inconsistent with most other error reporting. It should be unable to delete queue %d: %d I disagree. Reporting an error code is precisely the case where I think (%d) is valuable. A colon suggests that an understandable explanation follows, while an error code isn't that understandable. What I expect as a user is: Read failed: I/O error but: Read failed (-5) And I thought this was the usual convention, too, but you seem to disagree. And this: printk(KERN_ERR %s: context size (%u) exceeds payload doesn't need the parens Agreed. Here: printk(hardirqs last enabled at (%u): , curr-hardirq_enable_event); print_ip_sym(curr-hardirq_enable_ip); printk(hardirqs last disabled at (%u): , curr-hardirq_disable_event); print_ip_sym(curr-hardirq_disable_ip); printk(softirqs last enabled at (%u): , curr-softirq_enable_event); print_ip_sym(curr-softirq_enable_ip); printk(softirqs last disabled at (%u): , curr-softirq_disable_event); print_ip_sym(curr-softirq_disable_ip); all the parens are just illogical and should be removed. Agreed. Here: xlog_warn(XFS: %s: unrecognised log version (%d)., __FUNCTION__, INT_GET(rhead-h_version, ARCH_CONVERT)); should use unrecognised log version: %d I'm fine both ways. This: printk(KERN_ERR udf: unknown compression code (%d) stri=%s\n, cmp_id, ocu_i-u_name); should use colon as well. So in fact a large number of the instances I see in there are illogical and basically gramatically wrong and should be converted to use a colon. I would be fine having one or more explicit rules in CodingStyle on how log messages should be formatted, if you think it adds value. But a general statement don't do foo when it's sometimes just fine to do foo, doesn't add any value IMHO. -- Jean Delvare - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
Hi David, On Sat, 29 Sep 2007 15:30:15 -0700, David Brownell wrote: Let's kill it, please. (i.e., ACK) But ... why? What value could needless parens provide? By definition of needless, none. But the question is precisely whether the parentheses are always needless. The way your formulate your question suggests that you aren't actually interested in discussing the matter, so I am wondering why are you taking part at all. Who says that needless parens could provide value? Jean, which is why he submitted the patch. No, I didn't. I said that parentheses were sometimes useful. You said that parentheses were usually needless (which isn't that different, BTW.) Combining both statements into useless parentheses provide value is a logical flaw. And attributing it to me is dishonest. (...) I'd kind of think a change like this should have some positive motivation. My positive motivation was that this statement, as is, is more confusing than helpful. As a matter of fact, it led us to disagree on a patch you sent. And I would like contributors to focus on the more important parts of CodingStyle. If someone wants to improve the statement instead of removing it, that's fine with me, but not having the time to work on this, I proposed the removal. I would agree that line could be improved to say that messages should not be needlessly large. It does already: Make the messages concise, clear, and unambiguous. -- Jean Delvare - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
On Sat, 29 Sep 2007 03:51:56 PDT, Andrew Morton said: > Printing something like > > bytes remaining: 0x12 (18) > > is a quite logical thing to do, although pretty darm pointless. On the other hand, printing this: magic number: 0x2710 probably doesn't ring any bells, but if you changed that format to be "magic number: %x (%d)",foo,foo you'll almost certainly sit up and ask "Where the fsck did *that* come from?" (unless you're a *lot* better at doing hex conversions in your head than I). Yeah, *usually* pointless, but it has its place sometimes pgpAcOhi5ovzR.pgp Description: PGP signature
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
David Brownell <[EMAIL PROTECTED]> writes: >> > > Let's kill it, please. (i.e., ACK) >> > >> > But ... why? What value could needless parens provide? >> >> Who says that needless parens could provide value? > > Jean, which is why he submitted the patch. > You, implicitly, by acking a patch saying those parens are bad. > But not me ... I don't think this patch is merge-worthy. Would also add rules like "don't put parens around the word device" etc? There are countless silly things one could do, and we can't explicitly prohibit all of them. -- Måns Rullgård [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
On Sat, 29 Sep 2007 15:30:15 -0700 David Brownell wrote: > > > > Let's kill it, please. (i.e., ACK) > > > > > > But ... why? What value could needless parens provide? > > > > Who says that needless parens could provide value? > > Jean, which is why he submitted the patch. > You, implicitly, by acking a patch saying those parens are bad. > But not me ... I don't think this patch is merge-worthy. Thanks for clearing that up. Yes, you did have me confused. Sure, if something is needless, it doesn't provide value. So we disagree that some parens are needless. OK. > > > "Yet Another Subtle And Hard To Fix Source Of Bloat" is > > > not a plus. > > > > ISTM that we agree. > > Evidently not, since removing it would promote such bloat. > Explicitly removing disapproval is a form of approval... > > > > > I'd kind of think a change like this should have some > > > positive motivation. > > > > "Me, I agree that numbers in parens don't usually make sense for > > kernel messages." > > > > It's too trivial to be included in CodingStyle. > > So the reason to remove that guideline, and thereby encourage > proliferation of needless parens, is ... that some OTHER way > to get rid of them is working? Andrew listed some cases where parens make sense. He didn't say (and I don't say) [oops, parens] that they always make sense, but the statement in CodingStyle is too strict. Sometimes they make sense. > I would agree that line could be improved to say that messages > should not be needlessly large. Excess parens are one example; > wordiness is another (including printing 8 bit fields as 0x%08x), > and I'm sure there are more. --- ~Randy - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
> > > Let's kill it, please. (i.e., ACK) > > > > But ... why? What value could needless parens provide? > > Who says that needless parens could provide value? Jean, which is why he submitted the patch. You, implicitly, by acking a patch saying those parens are bad. But not me ... I don't think this patch is merge-worthy. > > "Yet Another Subtle And Hard To Fix Source Of Bloat" is > > not a plus. > > ISTM that we agree. Evidently not, since removing it would promote such bloat. Explicitly removing disapproval is a form of approval... > > I'd kind of think a change like this should have some > > positive motivation. > > "Me, I agree that numbers in parens don't usually make sense for > kernel messages." > > It's too trivial to be included in CodingStyle. So the reason to remove that guideline, and thereby encourage proliferation of needless parens, is ... that some OTHER way to get rid of them is working? I would agree that line could be improved to say that messages should not be needlessly large. Excess parens are one example; wordiness is another (including printing 8 bit fields as 0x%08x), and I'm sure there are more. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
On Sat, 29 Sep 2007 11:53:06 -0700 David Brownell wrote: > > From [EMAIL PROTECTED] Sat Sep 29 11:40:17 2007 > > > > Let's kill it, please. (i.e., ACK) > > But ... why? What value could needless parens provide? Who says that needless parens could provide value? > "Yet Another Subtle And Hard To Fix Source Of Bloat" is > not a plus. ISTM that we agree. > I'd kind of think a change like this should have some > positive motivation. "Me, I agree that numbers in parens don't usually make sense for kernel messages." It's too trivial to be included in CodingStyle. --- ~Randy - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
> From [EMAIL PROTECTED] Sat Sep 29 11:40:17 2007 > > Let's kill it, please. (i.e., ACK) But ... why? What value could needless parens provide? "Yet Another Subtle And Hard To Fix Source Of Bloat" is not a plus. I'd kind of think a change like this should have some positive motivation. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
On Sat, 29 Sep 2007 03:51:56 -0700 Andrew Morton wrote: > On Sat, 29 Sep 2007 12:25:30 +0200 Jean Delvare <[EMAIL PROTECTED]> wrote: > > > Remove a not particularly relevant rule from CodingStyle. > > Sometimes, printing numbers in parentheses doesn't add value, but in > > some (most?) cases it makes the message easier to read. As a matter of > > fact, this practice is widely used in the kernel: > > > > linux-2.6.23-rc8$ quilt grep -I '(%l*[du])' | wc -l > > 3166 > > linux-2.6.23-rc8$ > > > > Signed-off-by: Jean Delvare <[EMAIL PROTECTED]> > > --- > > Documentation/CodingStyle |2 -- > > 1 file changed, 2 deletions(-) > > > > --- linux-2.6.23-rc8.orig/Documentation/CodingStyle 2007-07-23 > > 16:44:32.0 +0200 > > +++ linux-2.6.23-rc8/Documentation/CodingStyle 2007-09-28 > > 23:53:23.0 +0200 > > @@ -638,8 +638,6 @@ concise, clear, and unambiguous. > > > > Kernel messages do not have to be terminated with a period. > > > > -Printing numbers in parentheses (%d) adds no value and should be avoided. > > - > > There are a number of driver model diagnostic macros in > > which you should use to make sure messages are matched to the right device > > and driver, and are tagged with the right level: dev_err(), dev_warn(), > > I wonder how that got there. http://linux.bkbits.net:8080/linux-2.6/?PAGE=cset=4034429a6JsOCMNXT3tTPAX1kX40bg Let's kill it, please. (i.e., ACK) --- ~Randy - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
> > -Printing numbers in parentheses (%d) adds no value and should be avoided. > > I wonder how that got there. Well, the only place that numbers "naturally" appear wrapped in parenthesis is tables of credits and debits... as a debit, sort of a literal "add no value" situation. ;) Oh, and for footnotes, in typographically challenged contexts. Me, I agree that numbers in parens don't usually make sense for kernel messages. - Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
On Sat, 29 Sep 2007 12:25:30 +0200 Jean Delvare <[EMAIL PROTECTED]> wrote:
> Remove a not particularly relevant rule from CodingStyle.
> Sometimes, printing numbers in parentheses doesn't add value, but in
> some (most?) cases it makes the message easier to read. As a matter of
> fact, this practice is widely used in the kernel:
>
> linux-2.6.23-rc8$ quilt grep -I '(%l*[du])' | wc -l
> 3166
> linux-2.6.23-rc8$
>
> Signed-off-by: Jean Delvare <[EMAIL PROTECTED]>
> ---
> Documentation/CodingStyle |2 --
> 1 file changed, 2 deletions(-)
>
> --- linux-2.6.23-rc8.orig/Documentation/CodingStyle 2007-07-23
> 16:44:32.0 +0200
> +++ linux-2.6.23-rc8/Documentation/CodingStyle2007-09-28
> 23:53:23.0 +0200
> @@ -638,8 +638,6 @@ concise, clear, and unambiguous.
>
> Kernel messages do not have to be terminated with a period.
>
> -Printing numbers in parentheses (%d) adds no value and should be avoided.
> -
> There are a number of driver model diagnostic macros in
> which you should use to make sure messages are matched to the right device
> and driver, and are tagged with the right level: dev_err(), dev_warn(),
I wonder how that got there.
Printing something like
bytes remaining: 0x12 (18)
is a quite logical thing to do, although pretty darm pointless.
otoh, looking at the various instances, we have lots of stuff like this:
printk(KERN_ERR "seq-oss: unable to delete queue %d (%d)\n", queue, rc);
which I would argue is wrong and is inconsistent with most other error
reporting. It should be
unable to delete queue %d: %d
And this:
printk(KERN_ERR "%s: context size (%u) exceeds payload "
doesn't need the parens
Here:
printk("hardirqs last enabled at (%u): ", curr->hardirq_enable_event);
print_ip_sym(curr->hardirq_enable_ip);
printk("hardirqs last disabled at (%u): ", curr->hardirq_disable_event);
print_ip_sym(curr->hardirq_disable_ip);
printk("softirqs last enabled at (%u): ", curr->softirq_enable_event);
print_ip_sym(curr->softirq_enable_ip);
printk("softirqs last disabled at (%u): ", curr->softirq_disable_event);
print_ip_sym(curr->softirq_disable_ip);
all the parens are just illogical and should be removed.
Here:
xlog_warn("XFS: %s: unrecognised log version (%d).",
__FUNCTION__, INT_GET(rhead->h_version, ARCH_CONVERT));
should use "unrecognised log version: %d"
This:
printk(KERN_ERR "udf: unknown compression code (%d) stri=%s\n",
cmp_id, ocu_i->u_name);
should use colon as well.
So in fact a large number of the instances I see in there are illogical and
basically gramatically wrong and should be converted to use a colon.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] CodingStyle: Printing numbers in parentheses is fine
Remove a not particularly relevant rule from CodingStyle. Sometimes, printing numbers in parentheses doesn't add value, but in some (most?) cases it makes the message easier to read. As a matter of fact, this practice is widely used in the kernel: linux-2.6.23-rc8$ quilt grep -I '(%l*[du])' | wc -l 3166 linux-2.6.23-rc8$ Signed-off-by: Jean Delvare <[EMAIL PROTECTED]> --- Documentation/CodingStyle |2 -- 1 file changed, 2 deletions(-) --- linux-2.6.23-rc8.orig/Documentation/CodingStyle 2007-07-23 16:44:32.0 +0200 +++ linux-2.6.23-rc8/Documentation/CodingStyle 2007-09-28 23:53:23.0 +0200 @@ -638,8 +638,6 @@ concise, clear, and unambiguous. Kernel messages do not have to be terminated with a period. -Printing numbers in parentheses (%d) adds no value and should be avoided. - There are a number of driver model diagnostic macros in which you should use to make sure messages are matched to the right device and driver, and are tagged with the right level: dev_err(), dev_warn(), -- Jean Delvare - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] CodingStyle: Printing numbers in parentheses is fine
Remove a not particularly relevant rule from CodingStyle. Sometimes, printing numbers in parentheses doesn't add value, but in some (most?) cases it makes the message easier to read. As a matter of fact, this practice is widely used in the kernel: linux-2.6.23-rc8$ quilt grep -I '(%l*[du])' | wc -l 3166 linux-2.6.23-rc8$ Signed-off-by: Jean Delvare [EMAIL PROTECTED] --- Documentation/CodingStyle |2 -- 1 file changed, 2 deletions(-) --- linux-2.6.23-rc8.orig/Documentation/CodingStyle 2007-07-23 16:44:32.0 +0200 +++ linux-2.6.23-rc8/Documentation/CodingStyle 2007-09-28 23:53:23.0 +0200 @@ -638,8 +638,6 @@ concise, clear, and unambiguous. Kernel messages do not have to be terminated with a period. -Printing numbers in parentheses (%d) adds no value and should be avoided. - There are a number of driver model diagnostic macros in linux/device.h which you should use to make sure messages are matched to the right device and driver, and are tagged with the right level: dev_err(), dev_warn(), -- Jean Delvare - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
On Sat, 29 Sep 2007 12:25:30 +0200 Jean Delvare [EMAIL PROTECTED] wrote: Remove a not particularly relevant rule from CodingStyle. Sometimes, printing numbers in parentheses doesn't add value, but in some (most?) cases it makes the message easier to read. As a matter of fact, this practice is widely used in the kernel: linux-2.6.23-rc8$ quilt grep -I '(%l*[du])' | wc -l 3166 linux-2.6.23-rc8$ Signed-off-by: Jean Delvare [EMAIL PROTECTED] --- Documentation/CodingStyle |2 -- 1 file changed, 2 deletions(-) --- linux-2.6.23-rc8.orig/Documentation/CodingStyle 2007-07-23 16:44:32.0 +0200 +++ linux-2.6.23-rc8/Documentation/CodingStyle2007-09-28 23:53:23.0 +0200 @@ -638,8 +638,6 @@ concise, clear, and unambiguous. Kernel messages do not have to be terminated with a period. -Printing numbers in parentheses (%d) adds no value and should be avoided. - There are a number of driver model diagnostic macros in linux/device.h which you should use to make sure messages are matched to the right device and driver, and are tagged with the right level: dev_err(), dev_warn(), I wonder how that got there. Printing something like bytes remaining: 0x12 (18) is a quite logical thing to do, although pretty darm pointless. otoh, looking at the various instances, we have lots of stuff like this: printk(KERN_ERR seq-oss: unable to delete queue %d (%d)\n, queue, rc); which I would argue is wrong and is inconsistent with most other error reporting. It should be unable to delete queue %d: %d And this: printk(KERN_ERR %s: context size (%u) exceeds payload doesn't need the parens Here: printk(hardirqs last enabled at (%u): , curr-hardirq_enable_event); print_ip_sym(curr-hardirq_enable_ip); printk(hardirqs last disabled at (%u): , curr-hardirq_disable_event); print_ip_sym(curr-hardirq_disable_ip); printk(softirqs last enabled at (%u): , curr-softirq_enable_event); print_ip_sym(curr-softirq_enable_ip); printk(softirqs last disabled at (%u): , curr-softirq_disable_event); print_ip_sym(curr-softirq_disable_ip); all the parens are just illogical and should be removed. Here: xlog_warn(XFS: %s: unrecognised log version (%d)., __FUNCTION__, INT_GET(rhead-h_version, ARCH_CONVERT)); should use unrecognised log version: %d This: printk(KERN_ERR udf: unknown compression code (%d) stri=%s\n, cmp_id, ocu_i-u_name); should use colon as well. So in fact a large number of the instances I see in there are illogical and basically gramatically wrong and should be converted to use a colon. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
-Printing numbers in parentheses (%d) adds no value and should be avoided. I wonder how that got there. Well, the only place that numbers naturally appear wrapped in parenthesis is tables of credits and debits... as a debit, sort of a literal add no value situation. ;) Oh, and for footnotes, in typographically challenged contexts. Me, I agree that numbers in parens don't usually make sense for kernel messages. - Dave - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
On Sat, 29 Sep 2007 03:51:56 -0700 Andrew Morton wrote: On Sat, 29 Sep 2007 12:25:30 +0200 Jean Delvare [EMAIL PROTECTED] wrote: Remove a not particularly relevant rule from CodingStyle. Sometimes, printing numbers in parentheses doesn't add value, but in some (most?) cases it makes the message easier to read. As a matter of fact, this practice is widely used in the kernel: linux-2.6.23-rc8$ quilt grep -I '(%l*[du])' | wc -l 3166 linux-2.6.23-rc8$ Signed-off-by: Jean Delvare [EMAIL PROTECTED] --- Documentation/CodingStyle |2 -- 1 file changed, 2 deletions(-) --- linux-2.6.23-rc8.orig/Documentation/CodingStyle 2007-07-23 16:44:32.0 +0200 +++ linux-2.6.23-rc8/Documentation/CodingStyle 2007-09-28 23:53:23.0 +0200 @@ -638,8 +638,6 @@ concise, clear, and unambiguous. Kernel messages do not have to be terminated with a period. -Printing numbers in parentheses (%d) adds no value and should be avoided. - There are a number of driver model diagnostic macros in linux/device.h which you should use to make sure messages are matched to the right device and driver, and are tagged with the right level: dev_err(), dev_warn(), I wonder how that got there. http://linux.bkbits.net:8080/linux-2.6/?PAGE=csetREV=4034429a6JsOCMNXT3tTPAX1kX40bg Let's kill it, please. (i.e., ACK) --- ~Randy - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
From [EMAIL PROTECTED] Sat Sep 29 11:40:17 2007 Let's kill it, please. (i.e., ACK) But ... why? What value could needless parens provide? Yet Another Subtle And Hard To Fix Source Of Bloat is not a plus. I'd kind of think a change like this should have some positive motivation. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
On Sat, 29 Sep 2007 11:53:06 -0700 David Brownell wrote: From [EMAIL PROTECTED] Sat Sep 29 11:40:17 2007 Let's kill it, please. (i.e., ACK) But ... why? What value could needless parens provide? Who says that needless parens could provide value? Yet Another Subtle And Hard To Fix Source Of Bloat is not a plus. ISTM that we agree. I'd kind of think a change like this should have some positive motivation. Me, I agree that numbers in parens don't usually make sense for kernel messages. It's too trivial to be included in CodingStyle. --- ~Randy - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
Let's kill it, please. (i.e., ACK) But ... why? What value could needless parens provide? Who says that needless parens could provide value? Jean, which is why he submitted the patch. You, implicitly, by acking a patch saying those parens are bad. But not me ... I don't think this patch is merge-worthy. Yet Another Subtle And Hard To Fix Source Of Bloat is not a plus. ISTM that we agree. Evidently not, since removing it would promote such bloat. Explicitly removing disapproval is a form of approval... I'd kind of think a change like this should have some positive motivation. Me, I agree that numbers in parens don't usually make sense for kernel messages. It's too trivial to be included in CodingStyle. So the reason to remove that guideline, and thereby encourage proliferation of needless parens, is ... that some OTHER way to get rid of them is working? I would agree that line could be improved to say that messages should not be needlessly large. Excess parens are one example; wordiness is another (including printing 8 bit fields as 0x%08x), and I'm sure there are more. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
On Sat, 29 Sep 2007 15:30:15 -0700 David Brownell wrote: Let's kill it, please. (i.e., ACK) But ... why? What value could needless parens provide? Who says that needless parens could provide value? Jean, which is why he submitted the patch. You, implicitly, by acking a patch saying those parens are bad. But not me ... I don't think this patch is merge-worthy. Thanks for clearing that up. Yes, you did have me confused. Sure, if something is needless, it doesn't provide value. So we disagree that some parens are needless. OK. Yet Another Subtle And Hard To Fix Source Of Bloat is not a plus. ISTM that we agree. Evidently not, since removing it would promote such bloat. Explicitly removing disapproval is a form of approval... I'd kind of think a change like this should have some positive motivation. Me, I agree that numbers in parens don't usually make sense for kernel messages. It's too trivial to be included in CodingStyle. So the reason to remove that guideline, and thereby encourage proliferation of needless parens, is ... that some OTHER way to get rid of them is working? Andrew listed some cases where parens make sense. He didn't say (and I don't say) [oops, parens] that they always make sense, but the statement in CodingStyle is too strict. Sometimes they make sense. I would agree that line could be improved to say that messages should not be needlessly large. Excess parens are one example; wordiness is another (including printing 8 bit fields as 0x%08x), and I'm sure there are more. --- ~Randy - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
David Brownell [EMAIL PROTECTED] writes: Let's kill it, please. (i.e., ACK) But ... why? What value could needless parens provide? Who says that needless parens could provide value? Jean, which is why he submitted the patch. You, implicitly, by acking a patch saying those parens are bad. But not me ... I don't think this patch is merge-worthy. Would also add rules like don't put parens around the word device etc? There are countless silly things one could do, and we can't explicitly prohibit all of them. -- Måns Rullgård [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CodingStyle: Printing numbers in parentheses is fine
On Sat, 29 Sep 2007 03:51:56 PDT, Andrew Morton said: Printing something like bytes remaining: 0x12 (18) is a quite logical thing to do, although pretty darm pointless. On the other hand, printing this: magic number: 0x2710 probably doesn't ring any bells, but if you changed that format to be magic number: %x (%d),foo,foo you'll almost certainly sit up and ask Where the fsck did *that* come from? (unless you're a *lot* better at doing hex conversions in your head than I). Yeah, *usually* pointless, but it has its place sometimes pgpAcOhi5ovzR.pgp Description: PGP signature
