Re: [PATCH] Fix a printf hook problem

[Ryan Arnold <ryan dot arnold at gmail dot com>]

On Sat, Feb 12, 2011 at 9:10 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> Yesterday and today I've worked on printf hooks support for libquadmath
> (i.e. support for printing __float128 IEEE754 quad values on
> x86_64/i386/ia64). ÂI'd like to use Q modifier for it, is anyone aware for
> other uses of such modifier that would be incompatible with it?
>
> I've used
> http://www.eglibc.org/cgi-bin/viewcvs.cgi/*checkout*/libdfp/trunk/printf_dfp.c?content-type=text%2Fplain&rev=12594
> as the source for this, but found a few issues in glibc and libdfp
> printf_dfp.c. ÂCurrent libquadmath patch is at http://gcc.gnu.org/PR47642

> And the libdfp bugs:
>
> 1) Related to 1) above, libdfp arginfo function currently returns
> Â 0 if it is not D/DD/H modifier, which means that whenever you
> Â use e/E/f/F/g/G/a/A specifier, if no modifier or some other modifier
> Â is used it assumes no argument needs to be consumed (similarly to %m),
> Â i.e. when you register libdfp suddenly "%e %Lg %f" will not work
> Â anymore.

Yes this is a bug which I'll fix.  I suppose I'll have to make sure
this fix is predicated on a version of GLIBC which takes -1 from an
arginfo function.

> 2) __printf_dfp doesn't return -2 if modifier is not D/DD/H, which
> Â means that say %e is printed as if it was _Decimal32 number
> Â (and because of 1) above random garbage is printed as such number).

Yes, the else case when checking if (info->user & mod_FOO) is to
default to _Decimal32, which is a naive assumption that __printf_dfp()
isn't called for non DFP modifiers.  I'll fix this.

> 3) #ifndef OPTION_EGLIBC_LOCALE_CODE
> Â char decimald;
> Â #endif
> Â const char *decimal;
> Â ...
> Â #else
> Â /* Hard-code values from 'C' locale. Â*/
> Â decimald = ".";
> Â decimal = &decimald;
> Â decimalwc.wc = L'.';
> Â #endif
> Â can't possible work right, assigning "." to a char? ÂSurely
> Â that was supposed to be decimal = "."; and kill decimald.

Yes.

> 4) libdfp doesn't have a way to unregister, which means if
> Â you say dlopen libdfp, call the register function and
> Â then dlclose it, next printf will likely segfault.

Yes, this is due to 4) in the GLIBC list above.

> 5) Maybe the TR requires it, but I find it surprising that
> Â a/A prints a decimal number instead of hexadecimal on
> Â for _Decimal*.

Unfortunately, much to my displeasure, the TR DID override a/A in one
of the later drafts.  It's now quite a complicated modifier.

Ryan S. Arnold