[U-Boot] Bug in fdt_fixup_fman_firmware

Hello!
In my opinion I found a bug in the function 'fdt_fixup_fman_firmware' (arch/powerpc/cpu/mpc85xx/fdt.c): on line 495 function 'simple_strtul' takes an hex number without the prefix '0x' and considers it a decimal. Here are two variants for correcting this bug, but I do not know what will be correct:
--- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -308,9 +308,9 @@ */ int setenv_hex(const char *varname, ulong value) { - char str[17]; + char str[19];
- sprintf(str, "%lx", value); + sprintf(str, "0x%lx", value); return setenv(varname, str); }
--- a/arch/powerpc/cpu/mpc85xx/fdt.c +++ b/arch/powerpc/cpu/mpc85xx/fdt.c @@ -492,7 +492,7 @@ if (!p) return;
- fmanfw = (struct qe_firmware *) simple_strtoul(p, NULL, 0); + fmanfw = (struct qe_firmware *) simple_strtoul(p, NULL, 16); if (!fmanfw) return;
I think that would be correct to patch cmd_nvedit.c
Nikolay Puzanov.

Dear Николай Пузанов,
In message CAKj6i9ipzbmuGxMYFGhsQUAoy3yz6pNtd6iJXGPk-1pS3oK34A@mail.gmail.com you wrote:
In my opinion I found a bug in the function 'fdt_fixup_fman_firmware' (arch/powerpc/cpu/mpc85xx/fdt.c): on line 495 function 'simple_strtul' takes an hex number without the prefix '0x' and considers it a decimal. Here are two variants for correcting this bug,
...
int setenv_hex(const char *varname, ulong value) {
- char str[17];
- char str[19];
- sprintf(str, "%lx", value);
- sprintf(str, "0x%lx", value);
NAK. We don't need all these 0x prefixes. Hex is the default number base in U-Boot, so this is redundant.
- fmanfw = (struct qe_firmware *) simple_strtoul(p, NULL, 0);
- fmanfw = (struct qe_firmware *) simple_strtoul(p, NULL, 16);
This looks like a reasonable fix to me.
Note: you should have added the MC85xx custodian on Cc: (done now).
Best regards,
Wolfgang Denk

--- a/arch/powerpc/cpu/mpc85xx/fdt.c +++ b/arch/powerpc/cpu/mpc85xx/fdt.c @@ -492,7 +492,7 @@ if (!p) return;
- fmanfw = (struct qe_firmware *) simple_strtoul(p, NULL, 0);
- fmanfw = (struct qe_firmware *) simple_strtoul(p, NULL, 16); if (!fmanfw) return;
Could you submit this second one as a proper patch?
Thanks, Andy
participants (3)
-
Andy Fleming
-
Wolfgang Denk
-
Николай Пузанов