Message ID | 20200605150324.460300-1-hjl.tools@gmail.com |
---|---|
State | Superseded |
Headers |
Return-Path: <libc-alpha-bounces@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4840F3851C3A; Fri, 5 Jun 2020 15:03:30 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4840F3851C3A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1591369410; bh=OQln0eRjARHIv0RmjDGFP8sZ75uO2Q3KwJjwPIVU53s=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=R7Gj6yV5UYRUhgoDCa0JS92EhiH7JuqvWKLhcrxJiFO+C1+44Czr37tOvjnYJ5LR9 Gd1tbJznVX4GCgx2uqE5QvHbuOqjSrdFYHOhBTbdrdSC4iOgGPRpda4b0O/L/xHwrY ai4fCb4t+K0lfK1/sJ6CUvAO9HxF8XNUqlL59o98= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-pl1-x641.google.com (mail-pl1-x641.google.com [IPv6:2607:f8b0:4864:20::641]) by sourceware.org (Postfix) with ESMTPS id 770223851C28 for <libc-alpha@sourceware.org>; Fri, 5 Jun 2020 15:03:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 770223851C28 Received: by mail-pl1-x641.google.com with SMTP id n9so3748257plk.1 for <libc-alpha@sourceware.org>; Fri, 05 Jun 2020 08:03:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-transfer-encoding; bh=OQln0eRjARHIv0RmjDGFP8sZ75uO2Q3KwJjwPIVU53s=; b=QYs46eJ/p06ctgJmKpB5yy6p1/Y3RQsYrW1XNNF0q5LdmiCpVzD0r505xiBoypgWxs o/0JhtDAbSWSxGP0AVGlVtkBQN1athTaFsNxrt3M1NkycWxAct3cRNXBZN6arFZdb+hU 60BA28hMZUJLVCvWQKM2R+E9ECqX4bAZhhFm7eRuKvhlH0uDBebVXu2SgA7m1HPI201j W0KDBEohrv2M0BdejeW6dsN4v2ZcM5qIqXS4uUY06U3yj93iirhfv0ZGqifGyAOVf/s7 ons+jlQR/v3Bn9Oc2GjTsH8GIaJiSI9Qj3lZA/BQt6WJzkKgc4dmZLtI8pwiUWNYg89w biEw== X-Gm-Message-State: AOAM53061m7iPZWVLaiz36I7Xhyf+QF66Nit9m3+yMJV9XIKlqysznwi 9HkRe1CSwI7M+n1FaYkEtrPAU9xH X-Google-Smtp-Source: ABdhPJzVzrkExmO1WUg2W8rt/IjtIQYIxka8rzdJSw52337PFmf/JXHq3cdsbrMzlPeoJLqxFvzHuA== X-Received: by 2002:a17:90a:fe92:: with SMTP id co18mr3559104pjb.69.1591369406169; Fri, 05 Jun 2020 08:03:26 -0700 (PDT) Received: from gnu-cfl-2.localdomain (c-69-181-90-243.hsd1.ca.comcast.net. [69.181.90.243]) by smtp.gmail.com with ESMTPSA id 140sm7619060pfy.95.2020.06.05.08.03.25 for <libc-alpha@sourceware.org> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 05 Jun 2020 08:03:25 -0700 (PDT) Received: from gnu-cfl-2.localdomain (localhost [IPv6:::1]) by gnu-cfl-2.localdomain (Postfix) with ESMTP id 33C131A023E for <libc-alpha@sourceware.org>; Fri, 5 Jun 2020 08:03:24 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH] Add "%d" support to _dl_debug_vdprintf Date: Fri, 5 Jun 2020 08:03:24 -0700 Message-Id: <20200605150324.460300-1-hjl.tools@gmail.com> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-13.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list <libc-alpha.sourceware.org> List-Unsubscribe: <http://sourceware.org/mailman/options/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help> List-Subscribe: <http://sourceware.org/mailman/listinfo/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=subscribe> From: "H.J. Lu via Libc-alpha" <libc-alpha@sourceware.org> Reply-To: "H.J. Lu" <hjl.tools@gmail.com> Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces@sourceware.org> |
Series |
Add "%d" support to _dl_debug_vdprintf
|
|
Commit Message
H.J. Lu
June 5, 2020, 3:03 p.m. UTC
"%d" will be used to print out signed value. --- elf/dl-misc.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)
Comments
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.
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. >
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. > >
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.
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;