Re: [U-Boot] Bug in fdt_fixup_fman_firmware

[Andy Fleming]

 --- 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[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.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] Bug in fdt_fixup_fman_firmware

[Wolfgang Denk]

Dear Николай Пузанов,

In message cakj6i9ipzbmugxmyfghsquaoy3yz6pntd6ijxgpk-1ps3ok...@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

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
When properly administered, vacations do not  diminish  productivity:
for every week you're away and get nothing done, there's another when
your boss is away and you get twice as much done.  -- Daniel B. Luten
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot