--- ./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);
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 <[email protected]>
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 <[email protected]>
-mike
Hi,
On 6/19/07, Paulo Marques <[email protected]> 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
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.
On Tuesday 19 June 2007, Satyam Sharma wrote:
> On 6/19/07, Paulo Marques <[email protected]> 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
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 - http://www.grupopie.com
"Nostalgia isn't what it used to be."
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 - http://www.grupopie.com
"God is real, unless declared integer."
>>> 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
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 - http://www.grupopie.com
"All I ask is a chance to prove that money can't make me happy."
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