diff mbox series

V2 [PATCH] Add "%d" support to _dl_debug_vdprintf

Message ID CAMe9rOqvSTX_UM=aAK31VYOHbxfJSiWUr7gN_bZ=BTvHA=OS7g@mail.gmail.com
State Superseded
Headers
Series V2 [PATCH] Add "%d" support to _dl_debug_vdprintf |

Commit Message

H.J. Lu June 9, 2020, 2:38 p.m. UTC 
  On Tue, Jun 9, 2020 at 7:18 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 05/06/2020 12:03, H.J. Lu via Libc-alpha wrote:
> > "%d" will be used to print out signed value.
>
> LGTM with some smalls nits below.
>
> > ---
> >  elf/dl-misc.c | 31 +++++++++++++++++++++++++++++--
> >  1 file changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/elf/dl-misc.c b/elf/dl-misc.c
> > index ab70481fda..c82c8ae6fa 100644
> > --- a/elf/dl-misc.c
> > +++ b/elf/dl-misc.c
> > @@ -167,6 +167,7 @@ _dl_debug_vdprintf (int fd, int tag_p, const char *fmt, va_list arg)
> >         switch (*fmt)
> >           {
> >             /* Integer formatting.  */
> > +         case 'd':
> >           case 'u':
> >           case 'x':
> >             {
>
> Ok.
>
> > @@ -179,11 +180,34 @@ _dl_debug_vdprintf (int fd, int tag_p, const char *fmt, va_list arg)
> >  #else
> >               unsigned long int num = va_arg (arg, unsigned int);
> >  #endif
> > +             bool negative = false;
> > +             if (*fmt == 'd')
> > +               {
> > +#if LONG_MAX != INT_MAX
> > +                 if (long_mod)
> > +                   {
> > +                     if ((long) num < 0)
>
> Full type specify on cast.

Fixed.

> > +                       negative = true;
> > +                   }
> > +                 else
> > +                   {
> > +                     if ((int) num < 0)> +                     {
> > +                         num = (unsigned int) num;
> > +                         negative = true;
> > +                       }
> > +                   }
> > +#else
> > +                 if ((int) num < 0)
> > +                   negative = true;
> > +#endif
> > +               }
> > +
> >               /* We use alloca() to allocate the buffer with the most
> >                  pessimistic guess for the size.  Using alloca() allows
> >                  having more than one integer formatting in a call.  */
> > -             char *buf = (char *) alloca (3 * sizeof (unsigned long int));
> > -             char *endp = &buf[3 * sizeof (unsigned long int)];
> > +             char *buf = (char *) alloca (1 + 3 * sizeof (unsigned long int));
> > +             char *endp = &buf[1 + 3 * sizeof (unsigned long int)];
>
> I think we can remove alloca here and use INT_STRLEN_BOUND (since the
> string is not '\0' bounded due the writev usage).  Something like:
>
>   char buf[INT_STRLEN_BOUND (unsigned long int)];
>   char *endp = &buf[INT_STRLEN_BOUND (unsigned long int)];

Fixed.

> >               char *cp = _itoa (num, endp, *fmt == 'x' ? 16 : 10, 0);
> >
> >               /* Pad to the width the user specified.  */
> > @@ -191,6 +215,9 @@ _dl_debug_vdprintf (int fd, int tag_p, const char *fmt, va_list arg)
> >                 while (endp - cp < width)
> >                   *--cp = fill;
> >
> > +             if (negative)
> > +               *--cp = '-';
> > +
> >               iov[niov].iov_base = cp;
> >               iov[niov].iov_len = endp - cp;
> >               ++niov;
> >
>
> Ok.

Here is the updated patch I am checking in.

Thanks.
diff mbox series

Patch

From 6174817fdd86eab9b0ae396dbf5cc9b99ab06bfb Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 5 Jun 2020 07:59:25 -0700
Subject: [PATCH] Add "%d" support to _dl_debug_vdprintf

"%d" will be used to print out signed value and replace alloca with
INT_STRLEN_BOUND.
---
 elf/dl-misc.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/elf/dl-misc.c b/elf/dl-misc.c
index ab70481fda..a88d6370bb 100644
--- a/elf/dl-misc.c
+++ b/elf/dl-misc.c
@@ -31,6 +31,7 @@ 
 #include <sys/stat.h>
 #include <sys/uio.h>
 #include <sysdep.h>
+#include <intprops.h>
 #include <_itoa.h>
 #include <dl-writev.h>
 #include <not-cancel.h>
@@ -167,6 +168,7 @@  _dl_debug_vdprintf (int fd, int tag_p, const char *fmt, va_list arg)
 	  switch (*fmt)
 	    {
 	      /* Integer formatting.  */
+	    case 'd':
 	    case 'u':
 	    case 'x':
 	      {
@@ -179,11 +181,31 @@  _dl_debug_vdprintf (int fd, int tag_p, const char *fmt, va_list arg)
 #else
 		unsigned long int num = va_arg (arg, unsigned int);
 #endif
-		/* We use alloca() to allocate the buffer with the most
-		   pessimistic guess for the size.  Using alloca() allows
-		   having more than one integer formatting in a call.  */
-		char *buf = (char *) alloca (3 * sizeof (unsigned long int));
-		char *endp = &buf[3 * sizeof (unsigned long int)];
+		bool negative = false;
+		if (*fmt == 'd')
+		  {
+#if LONG_MAX != INT_MAX
+		    if (long_mod)
+		      {
+			if ((long int) num < 0)
+			  negative = true;
+		      }
+		    else
+		      {
+			if ((int) num < 0)
+			  {
+			    num = (unsigned int) num;
+			    negative = true;
+			  }
+		      }
+#else
+		    if ((int) num < 0)
+		      negative = true;
+#endif
+		  }
+
+		char buf[INT_STRLEN_BOUND (long int)];
+		char *endp = &buf[INT_STRLEN_BOUND (long int)];
 		char *cp = _itoa (num, endp, *fmt == 'x' ? 16 : 10, 0);
 
 		/* Pad to the width the user specified.  */
@@ -191,6 +213,9 @@  _dl_debug_vdprintf (int fd, int tag_p, const char *fmt, va_list arg)
 		  while (endp - cp < width)
 		    *--cp = fill;
 
+		if (negative)
+		  *--cp = '-';
+
 		iov[niov].iov_base = cp;
 		iov[niov].iov_len = endp - cp;
 		++niov;
-- 
2.26.2