From: Andrew Morton <akpm@linux-foundation•org>
To: Chen Gang <gang.chen@asianux•com>
Cc: Peter Zijlstra <peterz@infradead•org>,
Rusty Russell <rusty@rustcorp•com.au>, Robin Holt <holt@sgi•com>,
athorlton@sgi•com, Al Viro <viro@zeniv•linux.org.uk>,
linux-next@vger•kernel.org
Subject: Re: [PATCH] kernel/panic.c: reduce 1 byte usage for print tainted buffer.
Date: Mon, 7 Oct 2013 18:25:18 -0700 [thread overview]
Message-ID: <20131007182518.e64c8596851bfb6b44279129@linux-foundation.org> (raw)
In-Reply-To: <52535B8D.5000200@asianux.com>
On Tue, 08 Oct 2013 09:10:37 +0800 Chen Gang <gang.chen@asianux•com> wrote:
> > - Requires that the two literal "Tainted: " strings be kept in sync.
> >
>
> Hmm... if more than 2, we should use a macro instead of, else (within
> 2), especially they are near by, we can still let it hard coded, I feel
> that will more straightful for readers and writers.
>
>
> > - Assumes that strlen("Not tainted") <= strlen("Tainted") +
> > ARRAY_SIZE(tnts). Which is true, but might not be if someone makes
> > changes...
> >
>
> Hmm... it use snprintf() instead of sprintf(), although I feel better
> using scnprintf() instead of.
>
> This string can be trucated, and scnprintf() is more suitable for this
> kind of string. And snprintf() is for the string which can not be
> truncated (will return the ideal length to notify the caller).
It's hardly a huge issue, but I'd do something along the lines of
--- a/kernel/panic.c~a
+++ a/kernel/panic.c
@@ -233,13 +233,16 @@ static const struct tnt tnts[] = {
*/
const char *print_tainted(void)
{
- static char buf[ARRAY_SIZE(tnts) + sizeof("Tainted: ")];
+ static const char tainted[] = "Tainted: ";
+ static const char not_tainted[] = "Not tainted: ";
+ static char buf[ARRAY_SIZE(tnts) +
+ max(sizeof(tainted), sizeof(not_tainted))];
if (tainted_mask) {
char *s;
int i;
- s = buf + sprintf(buf, "Tainted: ");
+ s = buf + sprintf(buf, tainted);
for (i = 0; i < ARRAY_SIZE(tnts); i++) {
const struct tnt *t = &tnts[i];
*s++ = test_bit(t->bit, &tainted_mask) ?
@@ -247,7 +250,7 @@ const char *print_tainted(void)
}
*s = 0;
} else
- snprintf(buf, sizeof(buf), "Not tainted");
+ snprintf(buf, sizeof(buf), not_tainted);
return buf;
}
Except that doesn't compile because of our fancy max().
Maybe we have a compile-time-evaluable max which does plain old
((a < b) ? b : a), not sure... I don't think it's worth bothering
about - leave it be.
next prev parent reply other threads:[~2013-10-08 1:25 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-05 15:50 [PATCH] kernel/panic.c: reduce 1 byte usage for print tainted buffer Chen Gang
2013-10-05 15:53 ` Chen Gang
2013-10-07 16:35 ` Alex Thorlton
2013-10-07 21:04 ` Chen Gang
2013-10-08 0:27 ` Andrew Morton
2013-10-08 0:32 ` Al Viro
2013-10-08 1:10 ` Chen Gang
2013-10-08 1:25 ` Andrew Morton [this message]
2013-10-08 1:45 ` Chen Gang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131007182518.e64c8596851bfb6b44279129@linux-foundation.org \
--to=akpm@linux-foundation$(echo .)org \
--cc=athorlton@sgi$(echo .)com \
--cc=gang.chen@asianux$(echo .)com \
--cc=holt@sgi$(echo .)com \
--cc=linux-next@vger$(echo .)kernel.org \
--cc=peterz@infradead$(echo .)org \
--cc=rusty@rustcorp$(echo .)com.au \
--cc=viro@zeniv$(echo .)linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox