public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: "Gabriel F. T. Gomes" <gabriel@inconstante.eti.br>
To: Florian Weimer <fweimer@redhat.com>
Cc: Joseph Myers <joseph@codesourcery.com>, <libc-alpha@sourceware.org>
Subject: Re: [PATCH 01/14] Prepare vfprintf to use __printf_fp/__printf_fphex with float128 arg
Date: Tue, 18 Dec 2018 13:37:00 -0000	[thread overview]
Message-ID: <20181218112739.47ad5968@tereshkova> (raw)
In-Reply-To: <871s6f2iei.fsf@oldenburg2.str.redhat.com>

On Tue, 18 Dec 2018, Florian Weimer wrote:

>* Gabriel F. T. Gomes:
>>
>> @@ -1887,7 +1930,12 @@ printf_positional (FILE *s, const CHAR_T *format, int readonly_format,
>>  	      (args_value[cnt].pa_user, ap_savep);
>>  	  }
>>  	else
>> -	  args_value[cnt].pa_long_double = 0.0;
>> +	  {
>> +	    args_value[cnt].pa_long_double = 0.0;
>> +#if __HAVE_FLOAT128_UNLIKE_LDBL
>> +	    args_value[cnt].pa_float128 = 0;
>> +#endif
>> +	  }  
>
>This bit doesn't look right to me because args_value[cnt] is a union.
>You need to assign to the right member, or perhaps zero-initialize using
>memset.

Hmm, the original code doesn't seem to be dealing with anything particular
to the long double type.  It seems that assigning to .pa_long_double,
rather than to other members, was an arbitrary decision.  Do you know
why .pa_long_double was chosen?

So, if this code is just zero-initializing the memory, do you think I
should zero-initialize the whole of args_value/argsbuf.data when it gets
expanded (see below)?  Then I could remove the else block entirely.

Like this:

diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c
index 61769e0ce1..17f1bae796 100644
--- a/stdio-common/vfprintf-internal.c
+++ b/stdio-common/vfprintf-internal.c
@@ -1789,10 +1789,11 @@ printf_positional (FILE *s, const CHAR_T *format, int readonly_format,
     if (!scratch_buffer_set_array_size (&argsbuf, nargs, bytes_per_arg))
       {
        done = -1;
        goto all_done;
       }
+    memset (argsbuf.data, 0, argsbuf.size);
     args_value = argsbuf.data;
     /* Set up the remaining two arrays to each point past the end of
        the prior array, since space for all three has been allocated
        now.  */
     args_size = &args_value[nargs].pa_int;
@@ -1884,12 +1885,10 @@ printf_positional (FILE *s, const CHAR_T *format, int readonly_format,
          {
            args_value[cnt].pa_user = alloca (args_size[cnt]);
            (*__printf_va_arg_table[args_type[cnt] - PA_LAST])
              (args_value[cnt].pa_user, ap_savep);
          }
-       else
-         args_value[cnt].pa_long_double = 0.0;
        break;
       case -1:
        /* Error case.  Not all parameters appear in N$ format
           strings.  We have no way to determine their type.  */
        assert ((mode_flags & PRINTF_FORTIFY) != 0);

  reply	other threads:[~2018-12-18 13:27 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-21  2:10 [PATCH 00/14] Functions with format string for IEEE128 on powercpc64le Gabriel F. T. Gomes
2018-06-21  2:10 ` [PATCH 03/14] Add internal implementations for argp.h, err.h, and error.h functions Gabriel F. T. Gomes
2018-06-21 21:36   ` Joseph Myers
2018-06-21  2:10 ` [PATCH 01/14] Prepare vfprintf to use __printf_fp/__printf_fphex with float128 arg Gabriel F. T. Gomes
2018-06-21 21:22   ` Joseph Myers
2018-12-07 20:16     ` Gabriel F. T. Gomes
2018-12-18 11:18       ` Ping Re: [PATCH v2] " Gabriel F. T. Gomes
2018-12-18 12:13       ` [PATCH 01/14] " Florian Weimer
2018-12-18 13:37         ` Gabriel F. T. Gomes [this message]
2018-12-19 15:57           ` Florian Weimer
2018-06-21  2:10 ` [PATCH 02/14] Prepare vfscanf to use __strtof128_internal Gabriel F. T. Gomes
2018-06-21 21:35   ` Joseph Myers
2018-06-21 21:39     ` Zack Weinberg
2018-12-07 20:16     ` Gabriel F. T. Gomes
2018-06-21  2:11 ` [PATCH 12/14] ldbl-128ibm-compat: Add err.h functions Gabriel F. T. Gomes
2018-06-21  2:11 ` [PATCH 04/14] ldbl-128ibm-compat: Add regular character printing functions Gabriel F. T. Gomes
2018-06-21 21:03   ` Joseph Myers
2018-06-21  2:11 ` [PATCH 08/14] ldbl-128ibm-compat: Test double values Gabriel F. T. Gomes
2018-06-21  2:11 ` [PATCH 05/14] ldbl-128ibm-compat: Add wide character printing functions Gabriel F. T. Gomes
2018-06-21  2:11 ` [PATCH 11/14] ldbl-128ibm-compat: Add argp_error and argp_failure Gabriel F. T. Gomes
2018-06-21  2:11 ` [PATCH 09/14] ldbl-128ibm-compat: Add regular character scanning functions Gabriel F. T. Gomes
2018-06-21  2:11 ` [PATCH 13/14] ldbl-128ibm-compat: Add error.h functions Gabriel F. T. Gomes
2018-06-21  2:11 ` [PATCH 06/14] ldbl-128ibm-compat: Add regular character, fortified printing functions Gabriel F. T. Gomes
2018-06-21  2:11 ` [PATCH 14/14] ldbl-128ibm-compat: Add tests for err.h and error.h functions Gabriel F. T. Gomes
2018-06-21  2:11 ` [PATCH 10/14] ldbl-128ibm-compat: Add wide character scanning functions Gabriel F. T. Gomes
2018-06-21  2:11 ` [PATCH 07/14] ldbl-128ibm-compat: Add wide character, fortified printing functions Gabriel F. T. Gomes
2018-06-21 16:44 ` [PATCH 00/14] Functions with format string for IEEE128 on powercpc64le Joseph Myers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181218112739.47ad5968@tereshkova \
    --to=gabriel@inconstante.eti.br \
    --cc=fweimer@redhat.com \
    --cc=joseph@codesourcery.com \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).