Re: nl: line number decrement

[Pádraig Brady]

On 25/10/2020 08:39, KOBAYASHI Takashi wrote:
> Hello,
>
> I wrote a little patch for nl(1). Could you merge this mainstream?
> It reduces the line number with the "--line-increment(-i)" option and
> negative value. The "--starting-line-number(-v)" option has been allowed
> negative value but "-i" has not until now. It should be unified.
>
>
> diff --git a/src/nl.c b/src/nl.c
> index 8fe91f773..c61c81d5b 100644
> --- a/src/nl.c
> +++ b/src/nl.c
> @@ -508,7 +508,7 @@ main (int argc, char **argv)
>                                                0);
>             break;
>           case 'i':
> -          page_incr = xdectoimax (optarg, 1, INTMAX_MAX, "",
> +          page_incr = xdectoimax (optarg, INTMAX_MIN, INTMAX_MAX, "",
>                                     _("invalid line number increment"), 0);
>             break;
>           case 'p':
>
>
> I think this change in behavior was a bit hard a while ago because
> integer-overflow of the line number was verified with a comparison
> operator. However the bug had been fixed with the "INT_ADD_WRAPV" macro by
> Paul Eggert last year. It made the implementation of decrement safe.

This makes sense for consistency.

I'll also add another commit along the lines off the following,
to only fail when we need to output the overflowed number,
and add some tests around these limits.

thanks for the patch!
Pádraig

diff --git a/src/nl.c b/src/nl.c
index 8fe91f773..89c06a2ac 100644
--- a/src/nl.c
+++ b/src/nl.c
@@ -143,6 +143,9 @@ static char const *lineno_format = FORMAT_RIGHT_NOLZ;
 /* Current print line number.  */
 static intmax_t line_no;

+/* Whether the current line number has incremented past limits.  */
+static bool line_no_overflow;
+
 /* True if we have ever read standard input.  */
 static bool have_read_stdin;

@@ -275,10 +278,13 @@ build_type_arg (char const **typep,
 static void
 print_lineno (void)
 {
+  if (line_no_overflow)
+    die (EXIT_FAILURE, 0, _("line number overflow"));
+
   printf (lineno_format, lineno_width, line_no, separator_str);

   if (INT_ADD_WRAPV (line_no, page_incr, &line_no))
-    die (EXIT_FAILURE, 0, _("line number overflow"));
+    line_no_overflow = true;
 }

 /* Switch to a header section. */