* [PATCH] Add "%d" support to _dl_debug_vdprintf
@ 2020-06-05 15:03 H.J. Lu
2020-06-09 14:18 ` Adhemerval Zanella
0 siblings, 1 reply; 7+ messages in thread
From: H.J. Lu @ 2020-06-05 15:03 UTC (permalink / raw)
To: libc-alpha
"%d" will be used to print out signed value.
---
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':
{
@@ -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)
+ 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)];
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;
--
2.26.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Add "%d" support to _dl_debug_vdprintf
2020-06-05 15:03 [PATCH] Add "%d" support to _dl_debug_vdprintf H.J. Lu
@ 2020-06-09 14:18 ` Adhemerval Zanella
2020-06-09 14:38 ` V2 " H.J. Lu
2020-06-09 14:39 ` Adhemerval Zanella
0 siblings, 2 replies; 7+ messages in thread
From: Adhemerval Zanella @ 2020-06-09 14:18 UTC (permalink / raw)
To: libc-alpha, H.J. Lu
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:
char buf[INT_STRLEN_BOUND (unsigned long int)];
char *endp = &buf[INT_STRLEN_BOUND (unsigned long int)];
> 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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* V2 [PATCH] Add "%d" support to _dl_debug_vdprintf
2020-06-09 14:18 ` Adhemerval Zanella
@ 2020-06-09 14:38 ` H.J. Lu
2020-06-09 14:39 ` Adhemerval Zanella
1 sibling, 0 replies; 7+ messages in thread
From: H.J. Lu @ 2020-06-09 14:38 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: GNU C Library, Paul Eggert
[-- Attachment #1: Type: text/plain, Size: 3104 bytes --]
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.
--
H.J.
[-- Attachment #2: 0001-Add-d-support-to-_dl_debug_vdprintf.patch --]
[-- Type: text/x-patch, Size: 2210 bytes --]
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Add "%d" support to _dl_debug_vdprintf
2020-06-09 14:18 ` Adhemerval Zanella
2020-06-09 14:38 ` V2 " H.J. Lu
@ 2020-06-09 14:39 ` Adhemerval Zanella
2020-06-09 14:46 ` H.J. Lu
1 sibling, 1 reply; 7+ messages in thread
From: Adhemerval Zanella @ 2020-06-09 14:39 UTC (permalink / raw)
To: libc-alpha, H.J. Lu
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.
>
> char buf[INT_STRLEN_BOUND (unsigned long int)];
> char *endp = &buf[INT_STRLEN_BOUND (unsigned long int)];
>
>> 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.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Add "%d" support to _dl_debug_vdprintf
2020-06-09 14:39 ` Adhemerval Zanella
@ 2020-06-09 14:46 ` H.J. Lu
2020-06-09 16:19 ` Adhemerval Zanella
0 siblings, 1 reply; 7+ messages in thread
From: H.J. Lu @ 2020-06-09 14:46 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: GNU C Library
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:
gdb) p endp
$3 = 0x7fffffffd674 "\377\177"
(gdb) p buf
$4 = "X\345\377\367\377\177\000\000\371-2147483647"
(gdb) p &buf
$5 = (char (*)[20]) 0x7fffffffd660
(gdb) p 0x7fffffffd674 - 0x7fffffffd660
$6 = 20
(gdb) list
204 #endif
205 }
206
207 char buf[INT_STRLEN_BOUND (long int)];
208 char *endp = &buf[INT_STRLEN_BOUND (long int)];
209 char *cp = _itoa (num, endp, *fmt == 'x' ? 16 : 10, 0);
210
211 /* Pad to the width the user specified. */
212 if (width != -1)
213 while (endp - cp < width)
(gdb)
> >
> > char buf[INT_STRLEN_BOUND (unsigned long int)];
> > char *endp = &buf[INT_STRLEN_BOUND (unsigned long int)];
> >
> >> 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.
> >
--
H.J.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Add "%d" support to _dl_debug_vdprintf
2020-06-09 14:46 ` H.J. Lu
@ 2020-06-09 16:19 ` Adhemerval Zanella
2020-06-09 16:28 ` V3 " H.J. Lu
0 siblings, 1 reply; 7+ messages in thread
From: Adhemerval Zanella @ 2020-06-09 16:19 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* V3 [PATCH] Add "%d" support to _dl_debug_vdprintf
2020-06-09 16:19 ` Adhemerval Zanella
@ 2020-06-09 16:28 ` H.J. Lu
0 siblings, 0 replies; 7+ messages in thread
From: H.J. Lu @ 2020-06-09 16:28 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: GNU C Library
[-- Attachment #1: Type: text/plain, Size: 3084 bytes --]
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.
[-- Attachment #2: 0001-Add-d-support-to-_dl_debug_vdprintf.patch --]
[-- Type: text/x-patch, Size: 2023 bytes --]
From 561064eb588c0ca9caffc92c1c6af9cb7964d68b 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.
---
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..f9d1fd7993 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':
{
@@ -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 int) num < 0)
+ 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)];
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;
--
2.26.2
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-06-09 16:29 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-05 15:03 [PATCH] Add "%d" support to _dl_debug_vdprintf H.J. Lu
2020-06-09 14:18 ` Adhemerval Zanella
2020-06-09 14:38 ` V2 " H.J. Lu
2020-06-09 14:39 ` Adhemerval Zanella
2020-06-09 14:46 ` H.J. Lu
2020-06-09 16:19 ` Adhemerval Zanella
2020-06-09 16:28 ` V3 " H.J. Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).