[PATCH] malloc_simple: Remove usage of unsupported %zx format string

[PATCH] malloc_simple: Remove usage of unsupported %zx format string

[Pali Rohár]

Replace %zx by %lx and cast size_t to ulong.

U-Boot currently prints garbage debug output:
size=x, ptr=18, limit=18: 4002a000

With this change it prints correct debug data:
size=18, ptr=18, limit=2000: 4002a000

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 common/malloc_simple.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/malloc_simple.c b/common/malloc_simple.c
index 0267fb6bec87..67ee623850e0 100644
--- a/common/malloc_simple.c
+++ b/common/malloc_simple.c
@@ -23,7 +23,7 @@ static void *alloc_simple(size_t bytes, int align)
 
 	addr = ALIGN(gd->malloc_base + gd->malloc_ptr, align);
 	new_ptr = addr + bytes - gd->malloc_base;
-	log_debug("size=%zx, ptr=%lx, limit=%lx: ", bytes, new_ptr,
+	log_debug("size=%lx, ptr=%lx, limit=%lx: ", (ulong)bytes, new_ptr,
 		  gd->malloc_limit);
 	if (new_ptr > gd->malloc_limit) {
 		log_err("alloc space exhausted\n");
-- 
2.20.1

Re: [PATCH] malloc_simple: Remove usage of unsupported %zx format string

[Sean Anderson]

Hi Pali,

On 2/3/22 1:51 PM, Pali Rohár wrote:
> Replace %zx by %lx and cast size_t to ulong.
> 
> U-Boot currently prints garbage debug output:
> size=x, ptr=18, limit=18: 4002a000
> 
> With this change it prints correct debug data:
> size=18, ptr=18, limit=2000: 4002a000
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

This qualifier is implemented in vsprintf, but not tiny-printf,
and is widely used throughout the codebase. So perhaps a better
fix might be 

diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
index f661fc6505..ad25bb7383 100644
--- a/lib/tiny-printf.c
+++ b/lib/tiny-printf.c
@@ -229,7 +229,8 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va)
                                        ch = *fmt++;
                                }
                        }
-                       if (ch == 'l') {
+                       if (ch == 'l' ||
+                           (sizeof(size_t) >= sizeof(int) && ch == 'z')) {
                                ch = *(fmt++);
                                islong = true;
                        }
--

which is not completely correct (since tiny-printf doesn't
support long longs), but will address the core issue.

--Sean

Re: [PATCH] malloc_simple: Remove usage of unsupported %zx format string

[Sean Anderson]

On 2/3/22 2:25 PM, Sean Anderson wrote:
> Hi Pali,
> 
> On 2/3/22 1:51 PM, Pali Rohár wrote:
>> Replace %zx by %lx and cast size_t to ulong.
>> 
>> U-Boot currently prints garbage debug output:
>> size=x, ptr=18, limit=18: 4002a000
>> 
>> With this change it prints correct debug data:
>> size=18, ptr=18, limit=2000: 4002a000
>> 
>> Signed-off-by: Pali Rohár <pali@kernel.org>
> 
> This qualifier is implemented in vsprintf, but not tiny-printf,
> and is widely used throughout the codebase. So perhaps a better
> fix might be 
> 
> diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
> index f661fc6505..ad25bb7383 100644
> --- a/lib/tiny-printf.c
> +++ b/lib/tiny-printf.c
> @@ -229,7 +229,8 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va)
>                                         ch = *fmt++;
>                                 }
>                         }
> -                       if (ch == 'l') {
> +                       if (ch == 'l' ||
> +                           (sizeof(size_t) >= sizeof(int) && ch == 'z')) {
>                                 ch = *(fmt++);
>                                 islong = true;
>                         }
> --
> 
> which is not completely correct (since tiny-printf doesn't
> support long longs), but will address the core issue.

Actually, we probably need something more like

if (ch == 'z') {
	ch = *(fmt++);
	islong = sizeof(size_t) >= sizeof(int);
}

so that 32-bit arches still print the integer.

--Sean

Re: [PATCH] malloc_simple: Remove usage of unsupported %zx format string

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1991 bytes --]

On Thu, Feb 03, 2022 at 02:28:23PM -0500, Sean Anderson wrote:
> 
> 
> On 2/3/22 2:25 PM, Sean Anderson wrote:
> > Hi Pali,
> > 
> > On 2/3/22 1:51 PM, Pali Rohár wrote:
> >> Replace %zx by %lx and cast size_t to ulong.
> >> 
> >> U-Boot currently prints garbage debug output:
> >> size=x, ptr=18, limit=18: 4002a000
> >> 
> >> With this change it prints correct debug data:
> >> size=18, ptr=18, limit=2000: 4002a000
> >> 
> >> Signed-off-by: Pali Rohár <pali@kernel.org>
> > 
> > This qualifier is implemented in vsprintf, but not tiny-printf,
> > and is widely used throughout the codebase. So perhaps a better
> > fix might be 
> > 
> > diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
> > index f661fc6505..ad25bb7383 100644
> > --- a/lib/tiny-printf.c
> > +++ b/lib/tiny-printf.c
> > @@ -229,7 +229,8 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va)
> >                                         ch = *fmt++;
> >                                 }
> >                         }
> > -                       if (ch == 'l') {
> > +                       if (ch == 'l' ||
> > +                           (sizeof(size_t) >= sizeof(int) && ch == 'z')) {
> >                                 ch = *(fmt++);
> >                                 islong = true;
> >                         }
> > --
> > 
> > which is not completely correct (since tiny-printf doesn't
> > support long longs), but will address the core issue.
> 
> Actually, we probably need something more like
> 
> if (ch == 'z') {
> 	ch = *(fmt++);
> 	islong = sizeof(size_t) >= sizeof(int);
> }
> 
> so that 32-bit arches still print the integer.

Right, but then we grow tiny-printf on the boards that really need to be
super concerned about space.  We have typically done what Pali proposes
here before of make the subset of code that runs under tiny-printf use
more restrictive and possibly slightly less optimal format characters.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] malloc_simple: Remove usage of unsupported %zx format string

[Simon Glass]

Hi Pali,

On Thu, 3 Feb 2022 at 11:51, Pali Rohár <pali@kernel.org> wrote:
>
> Replace %zx by %lx and cast size_t to ulong.
>
> U-Boot currently prints garbage debug output:
> size=x, ptr=18, limit=18: 4002a000

Do you mean in SPL?

>
> With this change it prints correct debug data:
> size=18, ptr=18, limit=2000: 4002a000
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  common/malloc_simple.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

> diff --git a/common/malloc_simple.c b/common/malloc_simple.c
> index 0267fb6bec87..67ee623850e0 100644
> --- a/common/malloc_simple.c
> +++ b/common/malloc_simple.c
> @@ -23,7 +23,7 @@ static void *alloc_simple(size_t bytes, int align)
>
>         addr = ALIGN(gd->malloc_base + gd->malloc_ptr, align);
>         new_ptr = addr + bytes - gd->malloc_base;
> -       log_debug("size=%zx, ptr=%lx, limit=%lx: ", bytes, new_ptr,
> +       log_debug("size=%lx, ptr=%lx, limit=%lx: ", (ulong)bytes, new_ptr,
>                   gd->malloc_limit);
>         if (new_ptr > gd->malloc_limit) {
>                 log_err("alloc space exhausted\n");
> --
> 2.20.1
>

Regards,
Simon

Re: [PATCH] malloc_simple: Remove usage of unsupported %zx format string

[Pali Rohár]

On Thursday 03 February 2022 14:19:41 Simon Glass wrote:
> Hi Pali,
> 
> On Thu, 3 Feb 2022 at 11:51, Pali Rohár <pali@kernel.org> wrote:
> >
> > Replace %zx by %lx and cast size_t to ulong.
> >
> > U-Boot currently prints garbage debug output:
> > size=x, ptr=18, limit=18: 4002a000
> 
> Do you mean in SPL?

Yes, this is printed in 32-bit mvebu SPL.

> >
> > With this change it prints correct debug data:
> > size=18, ptr=18, limit=2000: 4002a000
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  common/malloc_simple.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> > diff --git a/common/malloc_simple.c b/common/malloc_simple.c
> > index 0267fb6bec87..67ee623850e0 100644
> > --- a/common/malloc_simple.c
> > +++ b/common/malloc_simple.c
> > @@ -23,7 +23,7 @@ static void *alloc_simple(size_t bytes, int align)
> >
> >         addr = ALIGN(gd->malloc_base + gd->malloc_ptr, align);
> >         new_ptr = addr + bytes - gd->malloc_base;
> > -       log_debug("size=%zx, ptr=%lx, limit=%lx: ", bytes, new_ptr,
> > +       log_debug("size=%lx, ptr=%lx, limit=%lx: ", (ulong)bytes, new_ptr,
> >                   gd->malloc_limit);
> >         if (new_ptr > gd->malloc_limit) {
> >                 log_err("alloc space exhausted\n");
> > --
> > 2.20.1
> >
> 
> Regards,
> Simon

Re: [PATCH] malloc_simple: Remove usage of unsupported %zx format string

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 444 bytes --]

On Thu, Feb 03, 2022 at 07:51:37PM +0100, Pali Rohár wrote:

> Replace %zx by %lx and cast size_t to ulong.
> 
> U-Boot currently prints garbage debug output:
> size=x, ptr=18, limit=18: 4002a000
> 
> With this change it prints correct debug data:
> size=18, ptr=18, limit=2000: 4002a000
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]