From: Paulo Marques <pmarques@grupopie.com>
To: Joe Perches <joe@perches.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Andrew Morton <akpm@linux-foundation.org>,
Nick Andrew <nick@nick-andrew.net>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Greg Kroah-Hartman <gregkh@suse.de>,
netdev@vger.kernel.org, Bjorn Helgaas <bjorn.helgaas@hp.com>
Subject: Re: [PATCH] vsprintf.c: remove stack variable ksym from
Date: Mon, 15 Mar 2010 15:01:43 +0000 [thread overview]
Message-ID: <4B9E4BD7.1000104@grupopie.com> (raw)
In-Reply-To: <1268510079.30289.69.camel@Joe-Laptop.home>
Joe Perches wrote:
> On Sat, 2010-03-13 at 09:44 -0800, Joe Perches wrote:
>> On Sat, 2010-03-13 at 07:35 -0800, Linus Torvalds wrote:
>>> On Fri, 12 Mar 2010, Andrew Morton wrote:
>>>> nice.
>>> But the kallsyms_lookup()/sprint_symbol() functions don't take a
>>> length parameter, so we have to do the worst-case thing (which itself has
>>> tons of unnecessary padding).
>> Perhaps a new snprint_symbol function with the
>> other kallsyms... functions changed as necessary.
>
> Perhaps something like this:
Just one minor nit:
[...]
>
> - *result = '\0';
> + if (size)
> + *result = '\0';
This test seems to be here to handle the "size == 0" case, but
>[...]
> +const char *kallsyms_lookup_n(unsigned long addr,
> + unsigned long *symbolsize,
> + unsigned long *offset,
> + char **modname, char *namebuf, size_t size)
> +{
> + if (size)
> + namebuf[size - 1] = 0;
> + namebuf[0] = 0;
here we seem to write the namebuf[0] even if "size == 0". So maybe both
assignments in this function should be inside the "if (size)" test.
Other than that, the patch looks good:
Reviewed-by: Paulo Marques <pmarques@grupopie.com>
--
Paulo Marques - www.grupopie.com
"As far as we know, our computer has never had an undetected error."
Weisert
WARNING: multiple messages have this Message-ID (diff)
From: Paulo Marques <pmarques@grupopie.com>
To: Joe Perches <joe@perches.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Andrew Morton <akpm@linux-foundation.org>,
Nick Andrew <nick@nick-andrew.net>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Greg Kroah-Hartman <gregkh@suse.de>,
netdev@vger.kernel.org, Bjorn Helgaas <bjorn.helgaas@hp.com>
Subject: Re: [PATCH] vsprintf.c: remove stack variable ksym from
Date: Mon, 15 Mar 2010 15:01:43 +0000 [thread overview]
Message-ID: <4B9E4BD7.1000104@grupopie.com> (raw)
In-Reply-To: <1268510079.30289.69.camel@Joe-Laptop.home>
Joe Perches wrote:
> On Sat, 2010-03-13 at 09:44 -0800, Joe Perches wrote:
>> On Sat, 2010-03-13 at 07:35 -0800, Linus Torvalds wrote:
>>> On Fri, 12 Mar 2010, Andrew Morton wrote:
>>>> nice.
>>> But the kallsyms_lookup()/sprint_symbol() functions don't take a
>>> length parameter, so we have to do the worst-case thing (which itself has
>>> tons of unnecessary padding).
>> Perhaps a new snprint_symbol function with the
>> other kallsyms... functions changed as necessary.
>
> Perhaps something like this:
Just one minor nit:
[...]
>
> - *result = '\0';
> + if (size)
> + *result = '\0';
This test seems to be here to handle the "size == 0" case, but
>[...]
> +const char *kallsyms_lookup_n(unsigned long addr,
> + unsigned long *symbolsize,
> + unsigned long *offset,
> + char **modname, char *namebuf, size_t size)
> +{
> + if (size)
> + namebuf[size - 1] = 0;
> + namebuf[0] = 0;
here we seem to write the namebuf[0] even if "size == 0". So maybe both
assignments in this function should be inside the "if (size)" test.
Other than that, the patch looks good:
Reviewed-by: Paulo Marques <pmarques@grupopie.com>
--
Paulo Marques - www.grupopie.com
"As far as we know, our computer has never had an undetected error."
Weisert
next prev parent reply other threads:[~2010-03-15 15:34 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-04 6:21 [PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf Joe Perches
2010-03-04 6:21 ` [PATCH 1/2] vsprintf: Recursive vsnprintf: Add "%pV", struct va_format Joe Perches
2010-03-04 6:21 ` [PATCH 2/2] device.h drivers/base/core.c Convert dev_<level> macros to functions Joe Perches
2010-03-05 0:56 ` Andrew Morton
2010-03-05 1:00 ` Andrew Morton
2010-03-05 1:00 ` Andrew Morton
2010-03-05 2:46 ` Joe Perches
2010-03-04 6:27 ` [PATCH 1/2] vsprintf: Recursive vsnprintf: Add "%pV", struct va_format Joe Perches
2010-03-04 6:27 ` [PATCH 2/2] device.h drivers/base/core.c Convert dev_<level> macros to functions Joe Perches
2010-03-04 22:38 ` [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf Andrew Morton
2010-03-04 23:06 ` Linus Torvalds
2010-03-06 21:36 ` Joe Perches
2010-03-06 22:03 ` Linus Torvalds
2010-03-06 22:30 ` Joe Perches
2010-03-06 22:52 ` Linus Torvalds
2010-03-06 22:57 ` Linus Torvalds
2010-03-06 23:35 ` Joe Perches
2010-03-06 23:46 ` Linus Torvalds
2010-03-06 23:48 ` Linus Torvalds
2010-03-06 23:57 ` Joe Perches
2010-03-06 23:58 ` Linus Torvalds
2010-03-07 1:10 ` [PATCH] vsprintf.c: Reduce sizeof struct printf_spec from 24 to 8 bytes Joe Perches
2010-03-07 2:03 ` Linus Torvalds
2010-03-07 2:24 ` Linus Torvalds
2010-03-07 2:33 ` [PATCH] vsprintf.c: Use noinline_for_stack Joe Perches
2010-03-08 23:39 ` Joe Perches
2010-03-13 0:25 ` Andrew Morton
2010-03-13 15:35 ` Linus Torvalds
2010-03-13 17:44 ` Joe Perches
2010-03-13 19:54 ` [PATCH] vsprintf.c: remove stack variable ksym from Joe Perches
2010-03-15 15:01 ` Paulo Marques [this message]
2010-03-15 15:01 ` Paulo Marques
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=4B9E4BD7.1000104@grupopie.com \
--to=pmarques@grupopie.com \
--cc=akpm@linux-foundation.org \
--cc=bjorn.helgaas@hp.com \
--cc=gregkh@suse.de \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nick@nick-andrew.net \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.