[PATCH] remove usage of memmem from scripts/kallsyms.c

[PATCH] remove usage of memmem from scripts/kallsyms.c

[Paulo Marques]

[-- 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);
 

Re: [PATCH] remove usage of memmem from scripts/kallsyms.c

[Mike Frysinger]

[-- 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 --]

Re: [PATCH] remove usage of memmem from scripts/kallsyms.c

[Satyam Sharma]

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

Re: [PATCH] remove usage of memmem from scripts/kallsyms.c

[Mike Frysinger]

[-- 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 --]

Re: [PATCH] remove usage of memmem from scripts/kallsyms.c

[Paulo Marques]

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."

Re: [PATCH] remove usage of memmem from scripts/kallsyms.c

[Segher Boessenkool]

>>> 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

Re: [PATCH] remove usage of memmem from scripts/kallsyms.c

[Paulo Marques]

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."

Re: [PATCH] remove usage of memmem from scripts/kallsyms.c

[Sam Ravnborg]

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

Re: [PATCH] remove usage of memmem from scripts/kallsyms.c

[Christoph Hellwig]

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.

Re: [PATCH] remove usage of memmem from scripts/kallsyms.c

[Paulo Marques]

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."