Re: [PATCH] Use __snprintf when available

[Paolo Carlini <pcarlini at unitus dot it>]

Ulrich Drepper wrote:

> On Tue, 2002-03-19 at 09:58, Paolo Carlini wrote:
>
> > 1- Minor, but annoying: I could not manage to use
> __attribute__((__unused__))
> > for function parameters. Indeed, the following example triggers a
> parse error
> > at line #1:
> >
> >     f(int __size __attribute__ ((__unused__)), int i)
> >     {
> >       int j __attribute__ ((__unused__));
> >       return i;
> >     }
>
> For C++?  You wouldn't use attribute then:
>
>    int
>    f(int, int i)
>    {
>      int j __attribute__ ((__unused__));
>      return i;
>    }
>
> Just avoid the parameter name.

The example is a contrieved, in that is only meant to show that, contrary to my
expectations, __attribute__((__unused__) cannot be used for function parameters. The
actual library code, is something like this, where the __size parameter depending on
_GLIBCPP_USE_C99 may or not be actually unused and we definitely need it, in general:

  // Convert numeric value of type _Tv to string and return length of string
  template<typename _Tv>
    int
    __convert_from_v(char* __out, const int __size, const char* __fmt, _Tv __v,
       const __c_locale&, int __prec = -1)
    {
      int __ret;
      const char* __old = setlocale(LC_ALL, "C");
      if (__prec >= 0)
#ifdef _GLIBCPP_USE_C99
 __ret = snprintf(__out, __size, __fmt, __prec, __v);
#else
        __ret = sprintf(__out, __fmt, __prec, __v);
#endif
      else
#ifdef _GLIBCPP_USE_C99
 __ret = snprintf(__out, __size, __fmt, __v);
#else
        __ret = sprintf(__out, __fmt, __v);
#endif
      setlocale(LC_ALL, __old);
      if (__size); // Only to suppress a warning unused when !C99.
      return __ret;
    }

How does this fit in your reasoning line?

> >          char __modl, _ValueT __v) const
> >         {
> >    // [22.2.2.2.2] Stage 1, numeric conversion to character.
> > !
> >    // Long enough for the max format spec.
> >    char __fbuf[16];
> >    _S_format_int(__io, __fbuf, __mod, __modl);
> > ! #ifdef _GLIBCPP_USE_C99
> > !  // First try a buffer perhaps big enough.
> > !  int __cs_size = 64;
> > !  char* __cs = static_cast<char*>(__builtin_alloca(__cs_size));
> > !  int __len = __convert_from_v(__cs, __cs_size, __fbuf, __v,
> _S_c_locale);
> > !  // If the buffer was not large enough, try again with the correct
> size.
> > !  if (__len >= __cs_size)
> > !    {
> > !      __cs_size = __len + 1;
> > !      __cs = static_cast<char*>(__builtin_alloca(__cs_size));
> > !      __len = __convert_from_v(__cs, __cs_size, __fbuf, __v,
> _S_c_locale);
> > !    }
>
> I've only looked briefly at the code so correct me if I'm wrong.   There
> is no arbitrary limit to the size of the generated output.  You start
> with 64 bytes but then allocate as much as snprintf tells you, right?
> The user can specify width and precision.

My opinion is that there are no problems. In fact, as far as precision is concerned:

 const int __max_digits = numeric_limits<_ValueT>::digits10 + 1;
 streamsize __prec = __io.precision();

 if (__prec > static_cast<streamsize>(__max_digits))
   __prec = static_cast<streamsize>(__max_digits);

As regards "width" is not dealt with here and the staging buffer must only be as large as
the size in chars of the (double, long double) longest number in fixed or floating format.
There is no risk of umbound grow. Indeed, 'til now we have allocated a buffer of the
needed size with:

 // Consider the possibility of long ios_base::fixed outputs
 const bool __fixed = __io.flags() & ios_base::fixed;
 const int __max_exp = numeric_limits<_ValueT>::max_exponent10;
 // ios_base::fixed outputs may need up to __max_exp+1 chars
 // for the integer part + up to __max_digits chars for the
 // fractional part + 3 chars for sign, decimal point, '\0'. On
 // the other hand, for non-fixed outputs __max_digits*3 chars
 // are largely sufficient.
 const int __cs_size = __fixed ? __max_exp + __max_digits + 4
                               : __max_digits * 3;
 char* __cs = static_cast<char*>(__builtin_alloca(__cs_size));

But everything is much cleaner, neat and averaging to smaller runtime footprint with
snprintf (as your glibc docs teached me ;-)

The other 2 cases are similar, we are actuallt able to upperbound the required size to
allocate but it can be often quite large, with no purpose.

Ok?

Ciao,
Paolo.