[PATCH 1/5] lib/vsprintf.c: make sure vsnprintf() never returns a negative value

[Simon Glass]

Hi Rasmus,

On Thu, 20 May 2021 at 04:05, Rasmus Villemoes
<rasmus.villemoes at prevas.dk> wrote:
>
> Most callers (or callers of callers, etc.) of vsnprintf() are not
> prepared for it to return a negative value.
>
> The only case where that can currently happen is %pD, and it's IMO
> more user-friendly to produce some output that clearly shows that some
> "impossible" thing happened instead of having the message completely
> ignored - or mishandled as for example log.c would currently do.
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
> ---
>  lib/vsprintf.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)

I think that is debatable. If we want the calling code to be fixed,
then it needs to get an error code back. Otherwise the error will be
apparent to the user but (perhaps) not ever debugged.

The definition of printf() allows for the possibility of a negative
return value. We should add a prototype to vsprintf.h to indicate this
as the only existing one (in exports.h) has no comment.


>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 9dc96c81c6..0050110683 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -310,7 +310,7 @@ static char *device_path_string(char *buf, char *end, void *dp, int field_width,
>
>         str = efi_dp_str((struct efi_device_path *)dp);
>         if (!str)
> -               return ERR_PTR(-ENOMEM);
> +               return string(buf, end, "<%pD:ENOMEM>", field_width, precision, flags);
>
>         buf = string16(buf, end, str, field_width, precision, flags);
>         efi_free_pool(str);
> @@ -631,8 +631,6 @@ repeat:
>                         str = pointer(fmt + 1, str, end,
>                                         va_arg(args, void *),
>                                         field_width, precision, flags);
> -                       if (IS_ERR(str))
> -                               return PTR_ERR(str);
>                         /* Skip all alphanumeric pointer suffixes */
>                         while (isalnum(fmt[1]))
>                                 fmt++;
> @@ -798,9 +796,6 @@ int printf(const char *fmt, ...)
>         i = vscnprintf(printbuffer, sizeof(printbuffer), fmt, args);
>         va_end(args);
>
> -       /* Handle error */
> -       if (i <= 0)
> -               return i;
>         /* Print the string */
>         puts(printbuffer);
>         return i;
> @@ -817,9 +812,6 @@ int vprintf(const char *fmt, va_list args)
>          */
>         i = vscnprintf(printbuffer, sizeof(printbuffer), fmt, args);
>
> -       /* Handle error */
> -       if (i <= 0)
> -               return i;
>         /* Print the string */
>         puts(printbuffer);
>         return i;
> --
> 2.29.2
>