diff mbox

[U-Boot] Bug in fdt_fixup_fman_firmware

Message ID CAKj6i9ipzbmuGxMYFGhsQUAoy3yz6pNtd6iJXGPk-1pS3oK34A@mail.gmail.com
State Rejected
Delegated to: Wolfgang Denk
Headers show

Commit Message

Николай Пузанов April 16, 2013, 2:38 p.m. UTC 
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:

I think that would be correct to patch cmd_nvedit.c

Nikolay Puzanov.

Comments

Wolfgang Denk April 16, 2013, 3:56 p.m. UTC | #1 
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
Andy Fleming June 18, 2013, 8:13 p.m. UTC | #2 
> --- 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
diff mbox

Patch

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