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

[H.J. Lu]

On Tue, Jun 9, 2020 at 9:19 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 09/06/2020 11:46, H.J. Lu wrote:
> > On Tue, Jun 9, 2020 at 7:39 AM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >>
> >>
> >> On 09/06/2020 11:18, Adhemerval Zanella 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.
> >>>
> >>>> +                      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:
> >>
> >> Sigh, you can't actually remove the alloca here.
> >
> > Why not?  It seems to work:
> >
>
> Because the iov uses the allocated buffer on the _dl_writev and it will
> an invalid pointer if we allocate on the stack without alloca.

Here is the updated patch with alloca.  I am checking it in.

-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-d-support-to-_dl_debug_vdprintf.patch
Type: text/x-patch
Size: 2023 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20200609/e0b7a39c/attachment.bin>