* [PATCH] remove usage of memmem from scripts/kallsyms.c
@ 2007-06-19 16:15 Paulo Marques
2007-06-19 17:43 ` Mike Frysinger
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Paulo Marques @ 2007-06-19 16:15 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, Mike Frysinger
[-- Attachment #1: Type: text/plain, Size: 417 bytes --]
The only in-kernel user of "memmem" is scripts/kallsyms.c and it only
uses it to find tokens that are 2 bytes in size. It is trivial to
replace it with a simple function that finds 2-byte tokens.
This should help users from systems that don't have the memmem GNU
extension available.
Signed-off-by: Paulo Marques <pmarques@grupopie.com>
--
Paulo Marques - www.grupopie.com
"667: The neighbor of the beast."
[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1082 bytes --]
--- ./scripts/kallsyms.c.orig 2007-06-08 12:55:49.000000000 +0100
+++ ./scripts/kallsyms.c 2007-06-08 13:19:52.000000000 +0100
@@ -378,6 +378,17 @@ static void build_initial_tok_table(void
table_cnt = pos;
}
+static void *find_token(unsigned char *str, int len, unsigned char *token)
+{
+ int i;
+
+ for (i = 0; i < len - 1; i++) {
+ if (str[i] == token[0] && str[i+1] == token[1])
+ return &str[i];
+ }
+ return NULL;
+}
+
/* replace a given token in all the valid symbols. Use the sampled symbols
* to update the counts */
static void compress_symbols(unsigned char *str, int idx)
@@ -391,7 +402,7 @@ static void compress_symbols(unsigned ch
p1 = table[i].sym;
/* find the token on the symbol */
- p2 = memmem(p1, len, str, 2);
+ p2 = find_token(p1, len, str);
if (!p2) continue;
/* decrease the counts for this symbol's tokens */
@@ -410,7 +421,7 @@ static void compress_symbols(unsigned ch
if (size < 2) break;
/* find the token on the symbol */
- p2 = memmem(p1, size, str, 2);
+ p2 = find_token(p1, size, str);
} while (p2);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] remove usage of memmem from scripts/kallsyms.c
2007-06-19 16:15 [PATCH] remove usage of memmem from scripts/kallsyms.c Paulo Marques
@ 2007-06-19 17:43 ` Mike Frysinger
2007-06-19 18:47 ` Satyam Sharma
2007-06-19 19:04 ` Christoph Hellwig
2 siblings, 0 replies; 10+ messages in thread
From: Mike Frysinger @ 2007-06-19 17:43 UTC (permalink / raw)
To: Paulo Marques; +Cc: Andrew Morton, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 597 bytes --]
On Tuesday 19 June 2007, Paulo Marques wrote:
> The only in-kernel user of "memmem" is scripts/kallsyms.c and it only
> uses it to find tokens that are 2 bytes in size. It is trivial to
> replace it with a simple function that finds 2-byte tokens.
>
> This should help users from systems that don't have the memmem GNU
> extension available.
>
> Signed-off-by: Paulo Marques <pmarques@grupopie.com>
sorry for not getting back to you in the previous thread ... this is a much
nicer patch than what i proposed and works for me :)
Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 827 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] remove usage of memmem from scripts/kallsyms.c
2007-06-19 16:15 [PATCH] remove usage of memmem from scripts/kallsyms.c Paulo Marques
2007-06-19 17:43 ` Mike Frysinger
@ 2007-06-19 18:47 ` Satyam Sharma
2007-06-20 4:33 ` Mike Frysinger
2007-06-19 19:04 ` Christoph Hellwig
2 siblings, 1 reply; 10+ messages in thread
From: Satyam Sharma @ 2007-06-19 18:47 UTC (permalink / raw)
To: Paulo Marques; +Cc: Andrew Morton, linux-kernel, Mike Frysinger
Hi,
On 6/19/07, Paulo Marques <pmarques@grupopie.com> wrote:
>
> The only in-kernel user of "memmem" is scripts/kallsyms.c and it only
> uses it to find tokens that are 2 bytes in size. It is trivial to
> replace it with a simple function that finds 2-byte tokens.
>
> This should help users from systems that don't have the memmem GNU
> extension available.
So we could remove the "#define _GNU_SOURCE" at the top
of scripts/kallsyms.c too, presumably? If not (i.e. if there are
more GNUisms left in that file anyway), then I'm not sure if we
really gain by the change.
Satyam
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] remove usage of memmem from scripts/kallsyms.c
2007-06-19 16:15 [PATCH] remove usage of memmem from scripts/kallsyms.c Paulo Marques
2007-06-19 17:43 ` Mike Frysinger
2007-06-19 18:47 ` Satyam Sharma
@ 2007-06-19 19:04 ` Christoph Hellwig
2007-06-20 11:53 ` Paulo Marques
2 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2007-06-19 19:04 UTC (permalink / raw)
To: Paulo Marques; +Cc: Andrew Morton, linux-kernel, Mike Frysinger
On Tue, Jun 19, 2007 at 05:15:56PM +0100, Paulo Marques wrote:
>
> The only in-kernel user of "memmem" is scripts/kallsyms.c and it only
> uses it to find tokens that are 2 bytes in size. It is trivial to
> replace it with a simple function that finds 2-byte tokens.
>
> This should help users from systems that don't have the memmem GNU
> extension available.
Please add a comment describing why it's there so that it's not ripped
out again by the first janitor looking over the code.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] remove usage of memmem from scripts/kallsyms.c
2007-06-19 18:47 ` Satyam Sharma
@ 2007-06-20 4:33 ` Mike Frysinger
2007-06-20 11:56 ` Paulo Marques
0 siblings, 1 reply; 10+ messages in thread
From: Mike Frysinger @ 2007-06-20 4:33 UTC (permalink / raw)
To: Satyam Sharma; +Cc: Paulo Marques, Andrew Morton, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 670 bytes --]
On Tuesday 19 June 2007, Satyam Sharma wrote:
> On 6/19/07, Paulo Marques <pmarques@grupopie.com> wrote:
> > The only in-kernel user of "memmem" is scripts/kallsyms.c and it only
> > uses it to find tokens that are 2 bytes in size. It is trivial to
> > replace it with a simple function that finds 2-byte tokens.
> >
> > This should help users from systems that don't have the memmem GNU
> > extension available.
>
> So we could remove the "#define _GNU_SOURCE" at the top
> of scripts/kallsyms.c too, presumably? If not (i.e. if there are
> more GNUisms left in that file anyway), then I'm not sure if we
> really gain by the change.
yes, i believe this is true
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 827 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] remove usage of memmem from scripts/kallsyms.c
2007-06-19 19:04 ` Christoph Hellwig
@ 2007-06-20 11:53 ` Paulo Marques
0 siblings, 0 replies; 10+ messages in thread
From: Paulo Marques @ 2007-06-20 11:53 UTC (permalink / raw)
To: Christoph Hellwig, Paulo Marques, Andrew Morton, linux-kernel,
Mike Frysinger
Christoph Hellwig wrote:
> On Tue, Jun 19, 2007 at 05:15:56PM +0100, Paulo Marques wrote:
>> The only in-kernel user of "memmem" is scripts/kallsyms.c and it only
>> uses it to find tokens that are 2 bytes in size. It is trivial to
>> replace it with a simple function that finds 2-byte tokens.
>>
>> This should help users from systems that don't have the memmem GNU
>> extension available.
>
> Please add a comment describing why it's there so that it's not ripped
> out again by the first janitor looking over the code.
I don't see why it would seem a good idea to replace a simple find_token
function that searches for 2 byte tokens with a call to memmem. So, I
think this is not something a janitor would do.
The call to memmem was actually a left-over from a previous algorithm
that used variable sized tokens. With fixed size, 2 byte tokens, having
a specialized function is probably more efficient anyway.
--
Paulo Marques - www.grupopie.com
"Nostalgia isn't what it used to be."
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] remove usage of memmem from scripts/kallsyms.c
2007-06-20 4:33 ` Mike Frysinger
@ 2007-06-20 11:56 ` Paulo Marques
2007-06-20 15:25 ` Segher Boessenkool
0 siblings, 1 reply; 10+ messages in thread
From: Paulo Marques @ 2007-06-20 11:56 UTC (permalink / raw)
To: Mike Frysinger; +Cc: Satyam Sharma, Andrew Morton, linux-kernel
Mike Frysinger wrote:
> On Tuesday 19 June 2007, Satyam Sharma wrote:
>> So we could remove the "#define _GNU_SOURCE" at the top
>> of scripts/kallsyms.c too, presumably? If not (i.e. if there are
>> more GNUisms left in that file anyway), then I'm not sure if we
>> really gain by the change.
>
> yes, i believe this is true
I only tried in on x86 with my toolchain and it works, but I don't know
if it is worth the risk of breaking someone's setup for virtually no gain...
--
Paulo Marques - www.grupopie.com
"God is real, unless declared integer."
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] remove usage of memmem from scripts/kallsyms.c
2007-06-20 11:56 ` Paulo Marques
@ 2007-06-20 15:25 ` Segher Boessenkool
2007-06-20 15:53 ` Paulo Marques
0 siblings, 1 reply; 10+ messages in thread
From: Segher Boessenkool @ 2007-06-20 15:25 UTC (permalink / raw)
To: Paulo Marques; +Cc: Mike Frysinger, Satyam Sharma, Andrew Morton, linux-kernel
>>> So we could remove the "#define _GNU_SOURCE" at the top
>>> of scripts/kallsyms.c too, presumably? If not (i.e. if there are
>>> more GNUisms left in that file anyway), then I'm not sure if we
>>> really gain by the change.
>> yes, i believe this is true
>
> I only tried in on x86 with my toolchain and it works, but I don't
> know if it is worth the risk of breaking someone's setup for virtually
> no gain...
With the memmem() removed, the code builds (and works)
fine on several non-GNU systems. It should be perfectly
safe to remove the _GNU_SOURCE. Perhaps you should
replace it with the proper POSIX_ define so it won't
be broken again?
Segher
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] remove usage of memmem from scripts/kallsyms.c
2007-06-20 15:25 ` Segher Boessenkool
@ 2007-06-20 15:53 ` Paulo Marques
2007-06-20 16:42 ` Sam Ravnborg
0 siblings, 1 reply; 10+ messages in thread
From: Paulo Marques @ 2007-06-20 15:53 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Mike Frysinger, Satyam Sharma, Andrew Morton, linux-kernel
Segher Boessenkool wrote:
>>>> So we could remove the "#define _GNU_SOURCE" at the top
>>>> of scripts/kallsyms.c too, presumably? If not (i.e. if there are
>>>> more GNUisms left in that file anyway), then I'm not sure if we
>>>> really gain by the change.
>>> yes, i believe this is true
>>
>> I only tried in on x86 with my toolchain and it works, but I don't
>> know if it is worth the risk of breaking someone's setup for virtually
>> no gain...
>
> With the memmem() removed, the code builds (and works)
> fine on several non-GNU systems. It should be perfectly
> safe to remove the _GNU_SOURCE.
You're right. I went back in history and it was me who introduced the
_GNU_SOURCE when I added the memmem too (shame on me). So, if it worked
fine before, there is no reason to not work now that memmem is removed.
So I can:
- send an incremental patch with just that line removed
- send a replacement patch
- just leave it for now and wait until I work on kallsyms again to
silently remove that line together with other changes
Andrew, what would you prefer?
--
Paulo Marques - www.grupopie.com
"All I ask is a chance to prove that money can't make me happy."
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] remove usage of memmem from scripts/kallsyms.c
2007-06-20 15:53 ` Paulo Marques
@ 2007-06-20 16:42 ` Sam Ravnborg
0 siblings, 0 replies; 10+ messages in thread
From: Sam Ravnborg @ 2007-06-20 16:42 UTC (permalink / raw)
To: Paulo Marques
Cc: Segher Boessenkool, Mike Frysinger, Satyam Sharma, Andrew Morton,
linux-kernel
On Wed, Jun 20, 2007 at 04:53:15PM +0100, Paulo Marques wrote:
> Segher Boessenkool wrote:
> >>>>So we could remove the "#define _GNU_SOURCE" at the top
> >>>>of scripts/kallsyms.c too, presumably? If not (i.e. if there are
> >>>>more GNUisms left in that file anyway), then I'm not sure if we
> >>>>really gain by the change.
> >>>yes, i believe this is true
> >>
> >>I only tried in on x86 with my toolchain and it works, but I don't
> >>know if it is worth the risk of breaking someone's setup for virtually
> >>no gain...
> >
> >With the memmem() removed, the code builds (and works)
> >fine on several non-GNU systems. It should be perfectly
> >safe to remove the _GNU_SOURCE.
>
> You're right. I went back in history and it was me who introduced the
> _GNU_SOURCE when I added the memmem too (shame on me). So, if it worked
> fine before, there is no reason to not work now that memmem is removed.
>
> So I can:
>
> - send an incremental patch with just that line removed
>
> - send a replacement patch
>
> - just leave it for now and wait until I work on kallsyms again to
> silently remove that line together with other changes
>
> Andrew, what would you prefer?
Please send a replacement patch to me.
I will carry it in the kbuild tree.
Sam
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2007-06-20 16:42 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-06-19 16:15 [PATCH] remove usage of memmem from scripts/kallsyms.c Paulo Marques
2007-06-19 17:43 ` Mike Frysinger
2007-06-19 18:47 ` Satyam Sharma
2007-06-20 4:33 ` Mike Frysinger
2007-06-20 11:56 ` Paulo Marques
2007-06-20 15:25 ` Segher Boessenkool
2007-06-20 15:53 ` Paulo Marques
2007-06-20 16:42 ` Sam Ravnborg
2007-06-19 19:04 ` Christoph Hellwig
2007-06-20 11:53 ` Paulo Marques
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).