Re: Ping. Re: [PATCH v5] Prepare vfprintf to use __printf_fp/__printf_fphex with float128 arg

["Gabriel F. T. Gomes" <gabriel at inconstante dot eti dot br>]

Hi, Adhemerval,

> > On Tue, May 21 2019, Gabriel F. T. Gomes wrote:
> >>
> >> Changes since v4:
> >>
> >>   - Replaced macros with inline functions.

While running some final tests, I noticed that this last change is
incorrect.  On x86_64, powerpc32, and maybe others, it will fail to
build (details below).

> >> +static void
> >> +parse_float_va_arg (struct printf_info *info, union printf_arg *the_arg,
> >> +		    int is_long_double, unsigned int mode_flags,
> >> +		    va_list *ap)
> >> +{
> >> +#if __HAVE_FLOAT128_UNLIKE_LDBL
> >> +  if (is_long_double && (mode_flags & PRINTF_LDBL_USES_FLOAT128) != 0)
> >> +    {
> >> +      info->is_binary128 = 1;
> >> +      the_arg->pa_float128 = va_arg (*ap, _Float128);
> >> +    }
> >> +  else
> >> +#endif
> >> +    {
> >> +      info->is_binary128 = 0;
> >> +      if (is_long_double)
> >> +	the_arg->pa_long_double = va_arg (*ap, long double);
> >> +      else
> >> +	the_arg->pa_double = va_arg (*ap, double);
> >> +    }
> >> +}
> >> +
> >>
> >> [...]
> >>
> >> -	    if (is_long_double)						      \
> >> -	      the_arg.pa_long_double = va_arg (ap, long double);	      \
> >> -	    else							      \
> >> -	      the_arg.pa_double = va_arg (ap, double);			      \
> >> +	    parse_float_va_arg (&info, &the_arg, is_long_double,	      \
> >> +				mode_flags, &ap);			      \

On x86_64 and powerpc32, I get the following errors with this version of
the patch:

  vfprintf-internal.c: In function ‘__vfprintf_internal’:
  vfprintf-internal.c:811:17: error: passing argument 5 of ‘parse_float_va_arg’ from incompatible pointer type [-Werror=incompatible-pointer-types]
       mode_flags, &ap);         \
                   ^
  vfprintf-internal.c:1674:4: note: in expansion of macro ‘process_arg’
      process_arg (((struct printf_spec *) NULL));
      ^~~~~~~~~~~
  vfprintf-internal.c:154:1: note: expected ‘__va_list_tag (*)[1]’ but argument is of type ‘__va_list_tag **’
   parse_float_va_arg (struct printf_info *info, union printf_arg *the_arg,
   ^~~~~~~~~~~~~~~~~~

After I got this, I tried to use 'va_copy' to correctly pass the va_list
(and its state) to 'parse_float_va_arg', but that doesn't work, because
'va_arg' might have been called an unknown number of times with 'ap'.

I'm sorry I didn't notice this before (my tests with v5 were not good
enough), but I think we should go back to v4 (except for the typo fix)
and use the macros that correctly call 'va_arg' on the already
initialized 'va_list'.

Does that sound reasonable to you?