remove usage of memmem from scripts/kallsyms.c
PATCH 1/1 remove usage of memmem from scripts/kallsyms.c
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.
Signature · 3 lines
Paulo Marques - www.grupopie.com "667: The neighbor of the beast."
scripts/kallsyms.c renamed from scripts/kallsyms.c.orig +13 -2
@@ -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 chif (size <2 )break ;/* find the token on the symbol */ - p2 =memmem (p1, size, str,2 ); + p2 =find_token (p1, size, str); }while (p2);
Quoted 8 lines · Paulo Marques
The only in-kernel user of "memmem" is
scripts/kallsyms.cand 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 :)
-mike
Hi,
Quoted 7 lines · Paulo Marques
The only in-kernel user of "memmem" is
scripts/kallsyms.cand 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
Quoted 5 lines · Satyam Sharma
Quoted 4 lines · Paulo Marques
The only in-kernel user of "memmem" is
scripts/kallsyms.cand 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.
Quoted 7 lines
Quoted 2 lines
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 ofscripts/kallsyms.ctoo, 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
Quoted 7 lines · Mike Frysinger
Quoted 4 lines · Satyam Sharma
So we could remove the "#define
_GNU_SOURCE" at the top ofscripts/kallsyms.ctoo, 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...
Signature · 3 lines
Paulo Marques - www.grupopie.com "God is real, unless declared integer."
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
So we could remove the "#define
_GNU_SOURCE" at the top ofscripts/kallsyms.ctoo, 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
Quoted 11 lines · Segher Boessenkool
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
So we could remove the "#define
_GNU_SOURCE" at the top ofscripts/kallsyms.ctoo, 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?
Signature · 3 lines
Paulo Marques - www.grupopie.com "All I ask is a chance to prove that money can't make me happy."
Quoted 21 lines · Paulo Marques
Quoted 6 lines · Segher Boessenkool
Quoted 6 lines
Quoted 5 lines
Quoted 4 lines
So we could remove the "#define
_GNU_SOURCE" at the top ofscripts/kallsyms.ctoo, 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
Quoted 7 lines
Quoted 3 lines
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_SOURCEwhen 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:
Quoted 8 lines
- 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 changesAndrew, what would you prefer?
Please send a replacement patch to me.
I will carry it in the kbuild tree.
Sam
Quoted 7 lines · Paulo Marques
The only in-kernel user of "memmem" is
scripts/kallsyms.cand 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.
Quoted 7 lines · Christoph Hellwig
Quoted 3 lines · Paulo Marques
The only in-kernel user of "memmem" is
scripts/kallsyms.cand 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.
Signature · 3 lines
Paulo Marques - www.grupopie.com "Nostalgia isn't what it used to be."