From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Thu, 20 May 2021 11:51:41 -0600 Subject: [PATCH 1/5] lib/vsprintf.c: make sure vsnprintf() never returns a negative value In-Reply-To: <20210520100528.322846-2-rasmus.villemoes@prevas.dk> References: <20210520100528.322846-1-rasmus.villemoes@prevas.dk> <20210520100528.322846-2-rasmus.villemoes@prevas.dk> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Rasmus, On Thu, 20 May 2021 at 04:05, Rasmus Villemoes 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 > --- > 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 >