[U-Boot] [PATCH] spl: don't use %.*s with CONFIG_USE_TINY_PRINTF

[Oded Gabbay]

On Tue, Jan 17, 2017 at 11:18 AM, Masahiro Yamada
<yamada.masahiro at socionext.com> wrote:
> 2017-01-17 17:06 GMT+09:00 Oded Gabbay <oded.gabbay at gmail.com>:
>> On Tue, Jan 17, 2017 at 9:30 AM, Masahiro Yamada
>> <yamada.masahiro at socionext.com> wrote:
>>> 2016-12-28 16:36 GMT+09:00 Oded Gabbay <oded.gabbay at gmail.com>:
>>>> In the tiny-printf implementation, there is no support for %.*s. This patch
>>>> checks if CONFIG_USE_TINY_PRINTF is defined and if so, prints a different
>>>> debug statement which doesn't use %.*s
>>>>
>>>> Signed-off-by: Oded Gabbay <oded.gabbay at gmail.com>
>>>> Cc: Simon Glass <sjg at chromium.org>
>>>> ---
>>>>  common/spl/spl.c | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>>>> index f7df834..7c4744d 100644
>>>> --- a/common/spl/spl.c
>>>> +++ b/common/spl/spl.c
>>>> @@ -115,9 +115,14 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
>>>>                 }
>>>>                 spl_image->os = image_get_os(header);
>>>>                 spl_image->name = image_get_name(header);
>>>> +#ifdef CONFIG_USE_TINY_PRINTF
>>>> +               debug("spl: payload image: %s load addr: 0x%x size: %d\n",
>>>> +                       spl_image->name, spl_image->load_addr, spl_image->size);
>>>> +#else
>>>>                 debug("spl: payload image: %.*s load addr: 0x%x size: %d\n",
>>>>                         (int)sizeof(spl_image->name), spl_image->name,
>>>>                         spl_image->load_addr, spl_image->size);
>>>> +#endif
>>>>         } else {
>>>>  #ifdef CONFIG_SPL_PANIC_ON_RAW_IMAGE
>>>
>>>
>>> Same here.
>>>
>>> Please do not patch around with CONFIG_USE_TINY_PRINTF.
>>>
>>> What you need to do is to fix tiny_printf() implementation.
>>>
>>>
>>> --
>>> Best Regards
>>> Masahiro Yamada
>>
>> ok, I can accept that but how would you like to fix it ?
>> I guess you don't want to copy the entire printf implementation,
>> otherwise what's the point in tiny-printf ?
>
>
> Right.   Supporting all the format specifiers will make
> tiny-printf much bigger, then we will lose the point of this function.
>
> Can we silently ignore specifiers that are not recognized by tiny_printf()?
>
That is the default behavior today, but the result of that behavior is
that many prints look awful, with a lot of missing information, such
as : "address () " , or  "payload image: load addr" (between payload
image: and load addr should be the name of the image). It really makes
the whole thing totally unusable.

I didn't like that behavior and that's why I wanted to change it. I
think there are two options:
1. Use only implemented specifiers like in my original patches.
2. Silently replace un-implemented specifiers with the most similar
implemented specifiers, like %X to %x, or %.s to %s

Thanks,
Oded


>
>> Do you think we should just silently treat %.s as %s inside
>> tiny-printf ? And take that approach to the other two patches I sent ?
>>
>
>
> --
> Best Regards
> Masahiro Yamada