bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: FAIL: seq.log


From: Paul Eggert
Subject: Re: FAIL: seq.log
Date: Sat, 03 Nov 2007 01:10:59 -0700
User-agent: Gnus/5.11 (Gnus v5.11) Emacs/22.1 (gnu/linux)

Andreas Schwab <address@hidden> writes:

> Since 10.95 is exactly halfway between 10.9
> and 11.0 both representations are equally valid, depending on the
> precision of the underlying floating point format.

Thanks for reporting this.  Jim worked around the problem by changing
the test to specify 10.94, but I think it's nicer to fix 'seq' instead
to do the right thing with cases like this.  While fixing this bug I
noticed and fixed another one, with %% in the formats.  Here's a patch
for both bugs (I got lazy and fixed them both instead of breaking it
up into two patches, sorry....)

2007-11-03  Paul Eggert  <address@hidden>

        Fix bug with "seq 10.8 0.1 10.95", plus another bug with %% in format.

        * src/seq.c (struct layout): New type.
        (long_double_format): New arg LAYOUT.  Fill it in.  Fix mishandling
        of %% in formats.
        (print_numbers): New arg LAYOUT.  Don't convert LAST to output format
        when deciding whether to go slightly past LAST.  Instead, convert
        X to output format and back.  This fixes a bug reported by
        Andreas Schwab in
        <http://lists.gnu.org/archive/html/bug-coreutils/2007-10/msg00237.html>
        where "seq 10.8 0.1 10.95" would output 11.0 on platforms where
        10.95 rounds to a value that prints as 11.0 when only one digit
        past the decimal point is asked for.
        (main): Compute layout, for benefit of print_numbers.
        * tests/misc/seq (float-3): Undo previous change, since the bug
        should be fixed now.
        (fmt-b): New test, for the %% bug.

diff --git a/src/seq.c b/src/seq.c
index d9420ab..d7d2521 100644
--- a/src/seq.c
+++ b/src/seq.c
@@ -120,6 +120,14 @@ struct operand
 };
 typedef struct operand operand;

+/* Description of what a number-generating format will generate.  */
+struct layout
+{
+  /* Number of bytes before and after the number.  */
+  size_t prefix_len;
+  size_t suffix_len;
+};
+
 /* Read a long double value from the command line.
    Return if the string is correct else signal error.  */

@@ -171,17 +179,22 @@ scan_arg (const char *arg)

 /* If FORMAT is a valid printf format for a double argument, return
    its long double equivalent, possibly allocated from dynamic
-   storage; otherwise, return NULL.  */
+   storage, and store into *LAYOUT a description of the output layout;
+   otherwise, return NULL.  */

 static char const *
-long_double_format (char const *fmt)
+long_double_format (char const *fmt, struct layout *layout)
 {
   size_t i;
-  size_t prefix_len;
+  size_t prefix_len = 0;
+  size_t suffix_len = 0;
+  size_t length_modifier_offset;
   bool has_L;

-  for (i = 0; ! (fmt[i] == '%' && fmt[i + 1] != '%'); i++)
-    if (! fmt[i])
+  for (i = 0; ! (fmt[i] == '%' && fmt[i + 1] != '%'); i += (fmt[i] == '%') + 1)
+    if (fmt[i])
+      prefix_len++;
+    else
       return NULL;

   i++;
@@ -193,20 +206,25 @@ long_double_format (char const *fmt)
       i += strspn (fmt + i, "0123456789");
     }

-  prefix_len = i;
+  length_modifier_offset = i;
   has_L = (fmt[i] == 'L');
   i += has_L;
   if (! strchr ("efgaEFGA", fmt[i]))
     return NULL;

-  for (i++; ! (fmt[i] == '%' && fmt[i + 1] != '%'); i++)
-    if (! fmt[i])
+  for (i++; ! (fmt[i] == '%' && fmt[i + 1] != '%'); i += (fmt[i] == '%') + 1)
+    if (fmt[i])
+      suffix_len++;
+    else
       {
        size_t format_size = i + 1;
        char *ldfmt = xmalloc (format_size + 1);
-       memcpy (ldfmt, fmt, prefix_len);
-       ldfmt[prefix_len] = 'L';
-       strcpy (ldfmt + prefix_len + 1, fmt + prefix_len + has_L);
+       memcpy (ldfmt, fmt, length_modifier_offset);
+       ldfmt[length_modifier_offset] = 'L';
+       strcpy (ldfmt + length_modifier_offset + 1,
+               fmt + length_modifier_offset + has_L);
+       layout->prefix_len = prefix_len;
+       layout->suffix_len = suffix_len;
        return ldfmt;
       }

@@ -217,7 +235,7 @@ long_double_format (char const *fmt)
    given or default stepping and format.  */

 static void
-print_numbers (char const *fmt,
+print_numbers (char const *fmt, struct layout layout,
               long double first, long double step, long double last)
 {
   long double i;
@@ -229,8 +247,8 @@ print_numbers (char const *fmt,

       if (step < 0 ? x < last : last < x)
        {
-         /* If we go one past the end, but that number prints the
-            same way "last" does, and prints differently from the
+         /* If we go one past the end, but that number prints as a
+            value equal to "last", and prints differently from the
             previous number, then print "last".  This avoids problems
             with rounding.  For example, with the x86 it causes "seq
             0 0.000001 0.000003" to print 0.000003 instead of
@@ -238,13 +256,15 @@ print_numbers (char const *fmt,

          if (i)
            {
-             char *x_str = NULL;
-             char *last_str = NULL;
-             if (asprintf (&x_str, fmt, x) < 0
-                 || asprintf (&last_str, fmt, last) < 0)
+             long double x_val;
+             char *x_str;
+             int x_strlen = asprintf (&x_str, fmt, x);
+             if (x_strlen < 0)
                xalloc_die ();
+             x_str[x_strlen - layout.suffix_len] = '\0';

-             if (STREQ (x_str, last_str))
+             if (xstrtold (x_str + layout.prefix_len, NULL, &x_val, c_strtold)
+                 && x_val == last)
                {
                  char *x0_str = NULL;
                  if (asprintf (&x0_str, fmt, x0) < 0)
@@ -258,7 +278,6 @@ print_numbers (char const *fmt,
                }

              free (x_str);
-             free (last_str);
            }

          break;
@@ -317,6 +336,7 @@ main (int argc, char **argv)
   operand first = { 1, 1, 0 };
   operand step = { 1, 1, 0 };
   operand last;
+  struct layout layout = { 0, 0 };

   /* The printf(3) format used for output.  */
   char const *format_str = NULL;
@@ -385,7 +405,7 @@ main (int argc, char **argv)

   if (format_str)
     {
-      char const *f = long_double_format (format_str);
+      char const *f = long_double_format (format_str, &layout);
       if (! f)
        {
          error (0, 0, _("invalid format string: %s"), quote (format_str));
@@ -418,7 +438,7 @@ format string may not be specified when printing equal 
width strings"));
   if (format_str == NULL)
     format_str = get_default_format (first, step, last);

-  print_numbers (format_str, first.value, step.value, last.value);
+  print_numbers (format_str, layout, first.value, step.value, last.value);

   exit (EXIT_SUCCESS);
 }
diff --git a/tests/misc/seq b/tests/misc/seq
index 1f3ec06..73f4133 100755
--- a/tests/misc/seq
+++ b/tests/misc/seq
@@ -47,7 +47,7 @@ my @Tests =

    ['float-1', qw(0.8 0.1 0.9),        {OUT => [qw(0.8 0.9)]}],
    ['float-2', qw(0.1 0.99 1.99),      {OUT => [qw(0.10 1.09)]}],
-   ['float-3', qw(10.8 0.1 10.94),     {OUT => [qw(10.8 10.9)]}],
+   ['float-3', qw(10.8 0.1 10.95),     {OUT => [qw(10.8 10.9)]}],
    ['float-4', qw(0.1 -0.1 -0.2),      {OUT => [qw(0.1 0.0 -0.1 -0.2)]},
     {OUT_SUBST => 's,^-0\.0$,0.0,'},
    ],
@@ -79,6 +79,7 @@ my @Tests =
    ['fmt-8',   qw(-f %0+.0f 1 2),    {OUT => [qw(+1 +2)]}],
    ['fmt-9',   '-f "% -3.0f"', qw(-1 0), {OUT => ['-1 ', ' 0 ']}],
    ['fmt-a',   '-f "% -.0f"',qw(-1 0), {OUT => ['-1', ' 0']}],
+   ['fmt-b',   qw(-f %%%g%% 1),        {OUT => ['%1%']}],
   );

 # Append a newline to each entry in the OUT array.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]