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

Hi Wolfgang,

On Tue, Oct 30, 2012 at 4:59 AM, Wolfgang Denk wd@denx.de wrote:

Dear Tom Rini,

In message 1351558398-6902-1-git-send-email-trini@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@ni.com Cc: Gerald Van Baren vanbaren@cideas.com Signed-off-by: Tom Rini trini@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