Re: [Grep-devel] [PATCH] grep: use 'j' printf length modifier for [u]intmax_t

[Zev Weiss]

On Mon, Oct 17, 2016 at 10:38:54AM -0700, Jim Meyering wrote:
> On Sun, Oct 16, 2016 at 11:43 PM, Paul Eggert <address@hidden> wrote:
> > Jim Meyering wrote:
> > > I found that gettext has been using %j in one of its
> > > tools for over two years
> >
> >
> > While encouraging, gettext's users are developers who tend to use more
> > up-to-date libraries, whereas grep is an end-user tool that is more likely
> > to run atop ancient libraries. And if the libraries don't work, failures
> > will be at run-time and plausibily will occur only with large inputs that
> > our test cases won't match.
>
> Good point.
>
> > Before installing this patch, how about if we run gl_PRINTF_SIZES_C99 at
> > configure-time, and have 'configure' fail on platforms where
> > $gl_cv_func_printf_sizes_c99 is not 'yes'? That would nip these sorts of
> > problems in the bud.
>
> grep already uses printf.m4, so perhaps all that's needed is a few
> lines near the end of configure.ac, e.g., like this:
>
> gl_PRINTF_SIZES_C99
> if test "$gl_cv_func_printf_sizes_c99" = yes; then
>  AC_DEFINE([HAVE_PRINTF_C99_SIZES], [1],
>    [Define to 1 if printf formats %j, %z, %t and %L work.])
> fi
>
> Zev, do you want to add that to your patch, #ifdef your new one-liner,
> with the rest in an #else block and test that?
>
> Then, we can think about removing the #else block at our leisure.

Okay, something like the attached then?

I have to say I'm a bit unsure about what this version will actually do 
though.  I'm certainly far from an expert on gnulib and its associated 
autoconf tooling, but from my reading of printf.m4 it appears it only 
conditionally tests for printf("%j") support (predicated on 
HAVE_STDINT_H_WITH_UINTMAX || HAVE_INTTYPES_H_WITH_UINTMAX).  On a 
system sufficiently old that neither of those macros is defined, 
couldn't we still end up compiling the new %j code even if the runtime 
libc doesn't support it?

Also: my initial patch was actually part of an effort to pick any 
low-hanging fruit I could find in simplifying grep's output code as 
preparatory work for adding output buffering to my multithreading patch 
set (which I am continuing to maintain/develop).  That in combination 
with the fact that I don't have access to a pre-C99 system to test 
things like this on means I actually might lean slightly toward keeping 
it as-is in preference to maintaining the existing code in an #ifdef 
block, but it's not a strong preference either way, so if the general 
consensus is toward using printf when possible that's OK with me.


Zev

0001-grep-use-j-intmax_t-printf-length-modifier-if-suppor.patch
Description: Text Data