[U-Boot] [PATCH] cmd_fdt.c: Use %p when printing pointers

Joe Hershberger joe.hershberger at gmail.com
Tue Oct 30 18:48:39 CET 2012


Hi Wolfgang,

On Tue, Oct 30, 2012 at 4:59 AM, Wolfgang Denk <wd at denx.de> wrote:
> Dear Tom Rini,
>
> In message <1351558398-6902-1-git-send-email-trini at ti.com> you wrote:
>> When putting pointers into a format string use %p to ensure that they
>> are printed correctly regardless of bitsize.  This fixes warnings on
>> sandbox on 64bit systems.
>>
>> Cc: Joe Hershberger <joe.hershberger at ni.com>
>> Cc: Gerald Van Baren <vanbaren at cideas.com>
>> Signed-off-by: Tom Rini <trini at ti.com>
>> ---
>>  common/cmd_fdt.c |    6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/common/cmd_fdt.c b/common/cmd_fdt.c
>> index a5e2cfc..f9acfc1 100644
>> --- a/common/cmd_fdt.c
>> +++ b/common/cmd_fdt.c
>> @@ -375,7 +375,7 @@ int do_fdt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
>>                                       /* Get address */
>>                                       char buf[11];
>>
>> -                                     sprintf(buf, "0x%08X", (uint32_t)nodep);
>> +                                     sprintf(buf, "0x%p", nodep);
>>                                       setenv(var, buf);
>
> This may put bogus data ("var=0x(null)") into the environment.
>
> I see two approaches to fix this:
>
> 1) Handle this locally, say like that:
>
>         char buf[11] = { '0', 0, };
>
>         if (nodep)
>                 sprintf(buf, "0x%p", nodep);
>
>         setenv(var, buf);
>
>    This is the solution with minimal global impact.

I think this solution is not needed.  In this particular case, we are
always printing the pointer to a member inside the fdt, so even if the
image is at 0, no pointer that we are printing will ever be at 0.
Therefore this is code that will never run and can be left out.

> 2) Fix the root cause: given that we have valid situations where we
>    may want to dereference a pointer pointing to address 0x0000,
>    I wonder if it would not actually be better (i. e. more correct) to
>    get rid of the
>
>         495 static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
>         496                 int field_width, int precision, int flags)
>         497 {
>         498         if (!ptr)
>         499                 return string(buf, end, "(null)", field_width, precision,
>         500                               flags);
>
>    special handling in "lib/vsprintf.c"
>
>    Would anybody shed any tears if we drop this?

Getting rid of this would be good in general IMO.  I never did
understand why printing "(null)" was better than "0".

-Joe


More information about the U-Boot mailing list