This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v5 1/3] Add strfromd, strfromf, and strfroml functions


On 10/04/2016 01:57 PM, Gabriel F. T. Gomes wrote:
> ISO/IEC TS 18661-1 adds several functions in the strfrom family to stdlib.
> This patch adds strfromd, strfromf, and strfroml.  This is being done in
> preparation for the new floating-point type, float128.  The added functions
> convert a floating-point value into a string, with configurable format.
> 
> 2016-06-23  Gabriel F. T. Gomes  <gftg@linux.vnet.ibm.com>

> diff --git a/NEWS b/NEWS
> index 0b2ca04..49d9631 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -53,6 +53,9 @@ Version 2.25
>  
>    - Classification macros: iscanonical, issubnormal, iszero.
>  
> +* The functions strfromd, strfromf, and strfroml, from ISO/IEC TS 18661-1:2014,
> +  are added to libc.  They convert a floating-point number into string.
> +
>  * The <sys/quota.h> header now includes the <linux/quota.h> header.  Support
>    for the Linux quota interface which predates kernel version 2.4.22 has
>    been removed.
> diff --git a/manual/arith.texi b/manual/arith.texi
> index a13c46f..fa3b0b2 100644
> --- a/manual/arith.texi
> +++ b/manual/arith.texi
> @@ -20,6 +20,7 @@ These functions are declared in the header files @file{math.h} and
>  * Complex Numbers::             The types.  Writing complex constants.
>  * Operations on Complex::       Projection, conjugation, decomposition.
>  * Parsing of Numbers::          Converting strings to numbers.
> +* Printing of Floats::          Converting floating-point numbers to strings.
>  * System V Number Conversion::  An archaic way to convert numbers to strings.
>  @end menu
>  
> @@ -2736,6 +2737,46 @@ which take an additional argument, the locale to use in conversion.
>  
>  See also @ref{Parsing of Integers}.
>  
> +@node Printing of Floats
> +@section Printing of Floats
> +
> +@pindex stdlib.h
> +The @samp{strfrom} functions are declared in @file{stdlib.h}.
> +
> +@comment stdlib.h
> +@comment ISO/IEC TS 18661-1
> +@deftypefun int strfromd (char *restrict @var{string}, size_t @var{size}, const char *restrict @var{format}, double @var{value})
> +@deftypefunx int strfromf (char *restrict @var{string}, size_t @var{size}, const char *restrict @var{format}, float @var{value})
> +@deftypefunx int strfroml (char *restrict @var{string}, size_t @var{size}, const char *restrict @var{format}, long double @var{value})
> +@safety{@prelim{}@mtsafe{@mtslocale{}}@asunsafe{@ascuheap{}}@acunsafe{@acsmem{}}}
> +@comment these functions depend on __printf_fp and __printf_fphex, which are
> +@comment AS-unsafe (ascuheap) and AC-unsafe (acsmem).
> +The functions: @code{strfromd} (``string-from-double''), @code{strfromf}

Unnecessary ":".

> +(``string-from-float''), and @code{strfroml} (``string-from-long-double'')
> +convert the floating-point number in @var{value} to a string of characters

"number @var{value}" (no "in"); the italicizing causes the name to
represent the value, so the way you have written it reads, "convert the
floating-point number in 21.9785 to a string".

> +and stores them into the area pointed to by @var{string}.  The conversion
> +writes at most @var{size} characters and respects the format specified by
> +@var{format}.
> +
> +The @var{format} string must start with the character @samp{%}.  An

I wouldn't format format here at all, I think.

> +optional precision follows, which starts with a period ,@samp{.}, and may

s/ ,@/, @/

> +be followed by a decimal integer, representing the precision.  If a decimal
> +integer is not specified after the period, the precision is taken to be
> +zero.  The character @samp{*} is not allowed.  Finally, the @var{format}

Wouldn't use @var here either.

> +string ends with one of the following conversion specifiers: @samp{a},
> +@samp{A}, @samp{e}, @samp{E}, @samp{f}, @samp{F}, @samp{g}, @samp{G}.  The
> +meaning of each conversion specifier is described at @ref{Table of Output
> +Conversions}.

I would rearrange the last sentence to not use @ref{} because it doesn't
render correctly across all the documentation types.  The shortest
approach would be something like, "..., @samp{G} (@pxref{Table of Output
Conversions})."  Starting with @xref{} could work too, though.

> +
> +The return of these functions is the number of characters that would have

"These functions return the number of characters..." or "The return
value of these functions is the number of characters..."

> +been written to @var{string} had @var{size} been sufficiently large, not
> +counting the terminating null character.  Thus, the null-terminated output
> +has been completely written if and only if the returned value is less than
> +@var{size}.
> +
> +These functions have been introduced by ISO/IEC TS 18661-1.

20-years later it might be nice if this didn't sound like it just
happened.  Perhaps "were introduced", which gives it a solid,
future-proofing past tense feel?

> +@end deftypefun
> +
>  @node System V Number Conversion
>  @section Old-fashioned System V number-to-string functions
>  

A good description, overall.  Thank you.

> diff --git a/stdlib/Makefile b/stdlib/Makefile
> index de3ecbb..3cacb8b 100644
> --- a/stdlib/Makefile
> +++ b/stdlib/Makefile
> @@ -46,6 +46,7 @@ routines	:=							      \
>  	drand48_r erand48_r lrand48_r nrand48_r mrand48_r jrand48_r	      \
>  	srand48_r seed48_r lcong48_r					      \
>  	drand48-iter							      \
> +	strfromf strfromd strfroml					      \
>  	strtol strtoul strtoll strtoull					      \
>  	strtol_l strtoul_l strtoll_l strtoull_l				      \
>  	strtof strtod strtold						      \
> @@ -122,6 +123,12 @@ CFLAGS-fmtmsg.c = -fexceptions
>  CFLAGS-strfmon.c = $(libio-mtsafe)
>  CFLAGS-strfmon_l.c = $(libio-mtsafe)
>  
> +# The strfrom class of functions call __printf_fp in order to convert the
> +# floating-point value to characters.  This requires the value of IO_MTSAFE_IO.
> +CFLAGS-strfromd.c = $(libio-mtsafe)
> +CFLAGS-strfromf.c = $(libio-mtsafe)
> +CFLAGS-strfroml.c = $(libio-mtsafe)
> +
>  CFLAGS-tst-bsearch.c = $(stack-align-test-flags)
>  CFLAGS-tst-qsort.c = $(stack-align-test-flags)
>  CFLAGS-tst-makecontext.c += -funwind-tables
> diff --git a/stdlib/Versions b/stdlib/Versions
> index 9c06b43..54416b7 100644
> --- a/stdlib/Versions
> +++ b/stdlib/Versions
> @@ -112,6 +112,10 @@ libc {
>    GLIBC_2.24 {
>      quick_exit;
>    }
> +  GLIBC_2.25 {
> +    # s*
> +    strfromd; strfromf; strfroml;
> +  }
>    GLIBC_PRIVATE {
>      # functions which have an additional interface since they are
>      # are cancelable.
> diff --git a/stdlib/bits/stdlib-ldbl.h b/stdlib/bits/stdlib-ldbl.h
> index acff499..0c8f87b 100644
> --- a/stdlib/bits/stdlib-ldbl.h
> +++ b/stdlib/bits/stdlib-ldbl.h
> @@ -20,6 +20,9 @@
>  # error "Never include <bits/stdlib-ldbl.h> directly; use <stdlib.h> instead."
>  #endif
>  
> +#define __GLIBC_INTERNAL_STARTING_HEADER_IMPLEMENTATION
> +#include <bits/libc-header-start.h>
> +
>  #ifdef	__USE_ISOC99
>  __BEGIN_NAMESPACE_C99
>  __LDBL_REDIR1_DECL (strtold, strtod)
> @@ -30,6 +33,10 @@ __END_NAMESPACE_C99
>  __LDBL_REDIR1_DECL (strtold_l, strtod_l)
>  #endif
>  
> +#if __GLIBC_USE (IEC_60559_BFP_EXT)
> +__LDBL_REDIR1_DECL (strfroml, strfromd)
> +#endif
> +
>  #ifdef __USE_MISC
>  __LDBL_REDIR1_DECL (qecvt, ecvt)
>  __LDBL_REDIR1_DECL (qfcvt, fcvt)
> diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
> index f0dc951..48f9a95 100644
> --- a/stdlib/stdlib.h
> +++ b/stdlib/stdlib.h
> @@ -21,7 +21,8 @@
>  
>  #ifndef	_STDLIB_H
>  
> -#include <features.h>
> +#define __GLIBC_INTERNAL_STARTING_HEADER_IMPLEMENTATION
> +#include <bits/libc-header-start.h>
>  
>  /* Get size_t, wchar_t and NULL from <stddef.h>.  */
>  #define		__need_size_t
> @@ -178,6 +179,21 @@ extern unsigned long long int strtoull (const char *__restrict __nptr,
>  __END_NAMESPACE_C99
>  #endif /* ISO C99 or use MISC.  */
>  
> +/* Convert a floating-point number to a string.  */
> +#if __GLIBC_USE (IEC_60559_BFP_EXT)
> +extern int strfromd (char *__dest, size_t __size, const char *__format,
> +		     double __f)
> +     __THROW __nonnull ((3));
> +
> +extern int strfromf (char *__dest, size_t __size, const char *__format,
> +		     float __f)
> +     __THROW __nonnull ((3));
> +
> +extern int strfroml (char *__dest, size_t __size, const char *__format,
> +		     long double __f)
> +     __THROW __nonnull ((3));
> +#endif
> +
>  
>  #ifdef __USE_GNU
>  /* The concept of one static locale per category is not very well
> diff --git a/stdlib/strfrom-skeleton.c b/stdlib/strfrom-skeleton.c
> new file mode 100644
> index 0000000..82b5e03
> --- /dev/null
> +++ b/stdlib/strfrom-skeleton.c
> @@ -0,0 +1,153 @@
> +/* Convert a floating-point number to string.
> +
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.

Oh man!  That looks so much better with the space between the
description and the copyright!  The description is obvious, and
readable---it jumps out at you instead of forcing you stop everything
you're doing when you notice a (C) and visually dig around looking to
see if a brief comment was included, smashed halfway out the top of the
file.

...unfortunately I don't think any other file does it like that.  :(

> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +/* Generic implementation for strfrom functions.  The implementation is generic
> +   for several floating-point types (e.g.: float, double), so that each
> +   function, such as strfromf and strfroml, share the same code, thus avoiding
> +   code duplication.  */
> +
> +#include <ctype.h>
> +#include "../libio/libioP.h"
> +#include "../libio/strfile.h"
> +#include <printf.h>
> +#include <string.h>
> +#include <locale/localeinfo.h>
> +
> +#define UCHAR_T char
> +#define L_(Str) Str
> +#define ISDIGIT(Ch) isdigit (Ch)
> +#include "stdio-common/printf-parse.h"
> +
> +int
> +STRFROM (char *dest, size_t size, const char *format, FLOAT f)
> +{
> +  _IO_strnfile sfile;
> +#ifdef _IO_MTSAFE_IO
> +  sfile.f._sbf._f._lock = NULL;
> +#endif
> +
> +  int done;
> +
> +  /* Single-precision values need to be stored in a double type, because
> +     __printf_fp_l and __printf_fphex do not accept the float type.  */
> +  union {
> +    double flt;
> +    FLOAT value;
> +  } fpnum;
> +  const void *fpptr;
> +  fpptr = &fpnum;
> +
> +  /* Variables to control the output format.  Default is [-]x.yyyyyy.  */
> +  int precision = -1;
> +  int specifier = 'f';
> +  struct printf_info info;
> +
> +  /* Single-precision values need to be converted into double-precision,
> +     because __printf_fp and __printf_fphex only accept double and long double
> +     as the floating-point argument.  */
> +  if (__builtin_types_compatible_p (FLOAT, float))
> +    fpnum.flt = f;
> +  else
> +    fpnum.value = f;
> +
> +  /* Check if the first character in the format string is indeed the '%'
> +     character.  Otherwise, proceed with the default format.  */
> +  if (*format == '%')
> +    format++;
> +  else
> +    goto skip_format;
> +
> +  /* The optional precision specification always starts with a '.'.  If such
> +     character is present, read the precision.  */
> +  if (*format == '.')
> +    {
> +      format++;
> +
> +      /* Parse the precision.  */
> +      if (ISDIGIT (*format))
> +	precision = read_int (&format);
> +      /* If only the period is specified, the precision is taken as zero, as
> +	 described in ISO/IEC 9899:2011, section 7.21.6.1, 4th paragraph, 3rd
> +	 item.  */
> +      else
> +	precision = 0;
> +    }
> +
> +  /* Now there is only the conversion specifier to be read.  */
> +  switch (*format)
> +    {
> +    case 'a':
> +    case 'A':
> +    case 'e':
> +    case 'E':
> +    case 'f':
> +    case 'F':
> +    case 'g':
> +    case 'G':
> +      specifier = *format;
> +      break;
> +    default:
> +      specifier = 'f';
> +      break;
> +    }

Is it considered good style to break out of the default like that?

Actually, what I'm really wondering at this point is whether ignoring
invalid format specifiers is good form, instead of indicating some kind
of error (perhaps that's in the spec).  If we're going to skip_format
for specifiers that are set but don't begin with '%' and set precision=0
and specifier='f' for those that do but may be improperly formatted in
other ways, I think we should say so in the documentation.  A simple
addendum to the format specifier paragraph should suffice: "Invalid
format specifiers are silently ignored."

> +
> +skip_format:
> +
> +  /* The following code to prepare the virtual file has been adapted from the
> +     function _IO_vsnprintf from libio.  */
> +
> +  if (size == 0)
> +    {
> +    /* When size is zero, nothing is written and dest may be a null pointer.
> +       This is specified for snprintf in ISO/IEC 9899:2011, Section 7.21.6.5,
> +       in the second paragraph.  Thus, if size is zero, prepare to use the
> +       overflow buffer right from the start.  */
> +      dest = sfile.overflow_buf;
> +      size = sizeof (sfile.overflow_buf);
> +    }
> +
> +  /* Prepare the virtual string file.  */
> +  _IO_no_init (&sfile.f._sbf._f, _IO_USER_LOCK, -1, NULL, NULL);
> +  _IO_JUMPS (&sfile.f._sbf) = &_IO_strn_jumps;
> +  _IO_str_init_static_internal (&sfile.f, dest, size - 1, dest);
> +
> +  /* Prepare the format specification for printf_fp.  */
> +  memset (&info, '\0', sizeof (info));
> +
> +  /* The functions strfromd and strfromf pass a floating-point number with
> +     double precision to printf_fp, whereas strfroml passes a floating-point
> +     number with long double precision.  The following line informs printf_fp
> +     which type of floating-point number is being passed.  */
> +  info.is_long_double = __builtin_types_compatible_p (FLOAT, long double);
> +
> +  /* Set info according to the format string.  */
> +  info.prec = precision;
> +  info.spec = specifier;
> +
> +  if (info.spec != 'a' && info.spec != 'A')
> +    done = __printf_fp_l (&sfile.f._sbf._f, _NL_CURRENT_LOCALE, &info, &fpptr);
> +  else
> +    done = __printf_fphex (&sfile.f._sbf._f, &info, &fpptr);
> +
> +  /* Terminate the string.  */
> +  if (sfile.f._sbf._f._IO_buf_base != sfile.overflow_buf)
> +    *sfile.f._sbf._f._IO_write_ptr = '\0';
> +
> +  return done;
> +}
> diff --git a/stdlib/strfromd.c b/stdlib/strfromd.c
> new file mode 100644
> index 0000000..ddc010e
> --- /dev/null
> +++ b/stdlib/strfromd.c
> @@ -0,0 +1,23 @@
> +/* Definitions for strfromd.  Implementation in stdlib/strfrom-skeleton.c.
> +
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#define FLOAT		double
> +#define STRFROM		strfromd
> +
> +#include "strfrom-skeleton.c"
> diff --git a/stdlib/strfromf.c b/stdlib/strfromf.c
> new file mode 100644
> index 0000000..fa8c107
> --- /dev/null
> +++ b/stdlib/strfromf.c
> @@ -0,0 +1,23 @@
> +/* Definitions for strfromf.  Implementation in stdlib/strfrom-skeleton.c.
> +
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#define FLOAT		float
> +#define STRFROM		strfromf
> +
> +#include "strfrom-skeleton.c"
> diff --git a/stdlib/strfroml.c b/stdlib/strfroml.c
> new file mode 100644
> index 0000000..75592a4
> --- /dev/null
> +++ b/stdlib/strfroml.c
> @@ -0,0 +1,23 @@
> +/* Definitions for strfroml.  Implementation in stdlib/strfrom-skeleton.c.
> +
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#define FLOAT		long double
> +#define STRFROM		strfroml
> +
> +#include "strfrom-skeleton.c"

> diff --git a/sysdeps/ieee754/ldbl-opt/Makefile b/sysdeps/ieee754/ldbl-opt/Makefile
> index af08209..c922e2c 100644
> --- a/sysdeps/ieee754/ldbl-opt/Makefile
> +++ b/sysdeps/ieee754/ldbl-opt/Makefile
> @@ -24,6 +24,7 @@ libnldbl-calls = asprintf dprintf fprintf fscanf fwprintf fwscanf iovfscanf \
>  		 vdprintf_chk obstack_printf_chk obstack_vprintf_chk \
>  		 syslog syslog_chk vsyslog vsyslog_chk \
>  		 strfmon strfmon_l \
> +		 strfroml \
>  		 strtold strtold_l strtoldint wcstold wcstold_l wcstoldint \
>  		 qecvt qfcvt qgcvt qecvt_r qfcvt_r \
>  		 isinf isnan finite signbit scalb log2 lgamma_r ceil \
> diff --git a/sysdeps/ieee754/ldbl-opt/nldbl-strfroml.c b/sysdeps/ieee754/ldbl-opt/nldbl-strfroml.c
> new file mode 100644
> index 0000000..d6df69e
> --- /dev/null
> +++ b/sysdeps/ieee754/ldbl-opt/nldbl-strfroml.c
> @@ -0,0 +1,8 @@
> +#include "nldbl-compat.h"
> +
> +int
> +attribute_hidden
> +strfroml (char *dest, size_t size, const char *format, long double f)
> +{
> +  return strfromd (dest, size, format, f);
> +}

Rical


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]