2009-09-11 20:21:14

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH] kgdb: Replace strstr() by strchr() for single-character needles

Some versions of gcc replace calls to strstr() with single-character
"needle" string parameters by calls to strchr() behind our back.
This causes linking errors if strchr() is defined as an inline function
in <asm/string.h> (e.g. on m68k, which BTW doesn't have kgdb support).

Prevent this by explicitly calling strchr() instead.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
This is the single remaining case where strstr() is used with a
single-character needle. Cfr.
commit 0d03d59d9b31cd1e33b7e46a80b6fef66244b1f2 ("md: Fix "strchr"
[drivers/md/dm-log-userspace.ko] undefined!")
commit d8b0fb51ef1563c631d26cb649a5479b5cc4899c ("[PATCH] libertas link error
due to gcc `smartness'")

drivers/misc/kgdbts.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c
index e4ff50b..2a43d8c 100644
--- a/drivers/misc/kgdbts.c
+++ b/drivers/misc/kgdbts.c
@@ -885,16 +885,16 @@ static void kgdbts_run_tests(void)
int nmi_sleep = 0;
int i;

- ptr = strstr(config, "F");
+ ptr = strchr(config, 'F');
if (ptr)
fork_test = simple_strtol(ptr + 1, NULL, 10);
- ptr = strstr(config, "S");
+ ptr = strchr(config, 'S');
if (ptr)
do_sys_open_test = simple_strtol(ptr + 1, NULL, 10);
- ptr = strstr(config, "N");
+ ptr = strchr(config, 'N');
if (ptr)
nmi_sleep = simple_strtol(ptr+1, NULL, 10);
- ptr = strstr(config, "I");
+ ptr = strchr(config, 'I');
if (ptr)
sstep_test = simple_strtol(ptr+1, NULL, 10);

--
1.6.0.4

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


2009-09-12 15:17:18

by Jason Wessel

[permalink] [raw]
Subject: Re: [PATCH] kgdb: Replace strstr() by strchr() for single-character needles

I think the patch is fine and I will accept it into the kgdb tree for
pushing to Linus, but I did have a question.

How did the kgdbts.o end up getting compiled on m68k if there is no kgdb
support on m68k?

CONFIG_KGDB_TESTS should not be able to be set if there is no
CONFIG_KGDB set.

Thanks,
Jason.


Geert Uytterhoeven wrote:
> Some versions of gcc replace calls to strstr() with single-character
> "needle" string parameters by calls to strchr() behind our back.
> This causes linking errors if strchr() is defined as an inline function
> in <asm/string.h> (e.g. on m68k, which BTW doesn't have kgdb support).
>
> Prevent this by explicitly calling strchr() instead.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> This is the single remaining case where strstr() is used with a
> single-character needle. Cfr.
> commit 0d03d59d9b31cd1e33b7e46a80b6fef66244b1f2 ("md: Fix "strchr"
> [drivers/md/dm-log-userspace.ko] undefined!")
> commit d8b0fb51ef1563c631d26cb649a5479b5cc4899c ("[PATCH] libertas link error
> due to gcc `smartness'")
>
> drivers/misc/kgdbts.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c
> index e4ff50b..2a43d8c 100644
> --- a/drivers/misc/kgdbts.c
> +++ b/drivers/misc/kgdbts.c
> @@ -885,16 +885,16 @@ static void kgdbts_run_tests(void)
> int nmi_sleep = 0;
> int i;
>
> - ptr = strstr(config, "F");
> + ptr = strchr(config, 'F');
> if (ptr)
> fork_test = simple_strtol(ptr + 1, NULL, 10);
> - ptr = strstr(config, "S");
> + ptr = strchr(config, 'S');
> if (ptr)
> do_sys_open_test = simple_strtol(ptr + 1, NULL, 10);
> - ptr = strstr(config, "N");
> + ptr = strchr(config, 'N');
> if (ptr)
> nmi_sleep = simple_strtol(ptr+1, NULL, 10);
> - ptr = strstr(config, "I");
> + ptr = strchr(config, 'I');
> if (ptr)
> sstep_test = simple_strtol(ptr+1, NULL, 10);
>
>

2009-09-13 07:40:01

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] kgdb: Replace strstr() by strchr() for single-character needles

On Sat, Sep 12, 2009 at 17:16, Jason Wessel <[email protected]> wrote:
> I think the patch is fine and I will accept it into the kgdb tree for
> pushing to Linus, but I did have a question.
>
> How did the kgdbts.o end up getting compiled on m68k if there is no kgdb
> support on m68k?

It didn't. I was just grepping for the problematic strstr() case.

> Geert Uytterhoeven wrote:
>> Some versions of gcc replace calls to strstr() with single-character
>> "needle" string parameters by calls to strchr() behind our back.
>> This causes linking errors if strchr() is defined as an inline function
>> in <asm/string.h> (e.g. on m68k, which BTW doesn't have kgdb support).
>>
>> Prevent this by explicitly calling strchr() instead.
>>
>> Signed-off-by: Geert Uytterhoeven <[email protected]>
>> ---
>> This is the single remaining case where strstr() is used with a
>> single-character needle.  Cfr.
>> commit 0d03d59d9b31cd1e33b7e46a80b6fef66244b1f2 ("md: Fix "strchr"
>> [drivers/md/dm-log-userspace.ko] undefined!")
>> commit d8b0fb51ef1563c631d26cb649a5479b5cc4899c ("[PATCH] libertas link error
>> due to gcc `smartness'")

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds