cmd_fdt.c: Use %p when printing pointers

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 <[email protected]>

Cc Gerald Van Baren <[email protected]>

Signed-off-by Tom Rini <[email protected]>

---

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 <[email protected]>

Cc Gerald Van Baren <[email protected]>

Signed-off-by Tom Rini <[email protected]>

---
 common/cmd_fdt.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

common/cmd_fdt.c +0 -0

index a5e2cfc..f9acfc1 100644

Dear Tom Rini,

Quoted 16 lines · In message <[email protected]> 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 <[email protected]>

Cc Gerald Van Baren <[email protected]>

Signed-off-by Tom Rini <[email protected]>

---
 common/cmd_fdt.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

common/cmd_fdt.c +0 -0

index a5e2cfc..f9acfc1 100644

common/cmd_fdt.c int do_fdt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 379

Quoted 6 lines

common/cmd_fdt.c +1 -1

@@ -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.

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?

Quoted 2 lines

1) Handle this locally, say like that:

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.

Quoted 1 line

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".

Quoted 1 line · Wolfgang Denk

Dear Tom Rini,

Quoted 11 lines

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

Quoted 3 lines

special handling in "lib/vsprintf.c"

Would anybody shed any tears if we drop this?

I like that a lot better, gets my vote. I would suggest replacing the code with a comment that the (lack of) code intentionally does not print "(null)" or someone may put it back in in the future. :-/

The %p format of printf() would print a pointer to address null as "(null)". This makes sense in a real OS where a NULL pointer must never be dereferenced, but this is a bootloader, and there are cases where accessing the data at address null makes perfect sense.

Remove the special case in lib/vsprintf.c using "#if 0" with a comment to make clear this was an intentional change and to stop re-adding this code.

Signed-off-by Wolfgang Denk <[email protected]>

---

Hi Wolfgang,

Quoted 5 lines · Wolfgang Denk

The %p format of printf() would print a pointer to address null as "(null)". This makes sense in a real OS where a NULL pointer must never be dereferenced, but this is a bootloader, and there are cases where accessing the data at address null makes perfect sense.

Quoted 6 lines

Remove the special case in lib/vsprintf.c using "#if 0" with a comment to make clear this was an intentional change and to stop re-adding this code.

Signed-off-by Wolfgang Denk <[email protected]>

---

Acked-by Joe Hershberger <[email protected]>

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 <[email protected]>

Cc Gerald Van Baren <[email protected]>

Signed-off-by Tom Rini <[email protected]>