[src/trunk]: src/usr.bin/indent indent: parse int command line options strictly

[rillig <rillig%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/d9cd5e890081
branches:  trunk
changeset: 989869:d9cd5e890081
user:      rillig <rillig%NetBSD.org@localhost>
date:      Sun Oct 17 18:13:00 2021 +0000

description:
indent: parse int command line options strictly

On i386 and other platforms where LONG_MAX == INT_MAX, the test
t_errors/option_tabsize_very_large failed since the behavior on integer
overflow differs between ILP32 and LP64 platforms. Noticed by gson@.

Avoid this unintended difference by adding reasonable limits for each of
the integer options and by replacing atoi with strtol.

diffstat:

 tests/usr.bin/indent/opt.0.pro   |  10 +++++---
 tests/usr.bin/indent/t_errors.sh |  19 ++++++++++++----
 usr.bin/indent/args.c            |  44 ++++++++++++++++++++++++---------------
 usr.bin/indent/indent.c          |   8 +-----
 4 files changed, 49 insertions(+), 32 deletions(-)

diffs (232 lines):

diff -r 483a4aab94fa -r d9cd5e890081 tests/usr.bin/indent/opt.0.pro
--- a/tests/usr.bin/indent/opt.0.pro    Sun Oct 17 17:23:59 2021 +0000
+++ b/tests/usr.bin/indent/opt.0.pro    Sun Oct 17 18:13:00 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: opt.0.pro,v 1.2 2021/10/13 23:33:52 rillig Exp $ */
+/* $NetBSD: opt.0.pro,v 1.3 2021/10/17 18:13:00 rillig Exp $ */
 /* $FreeBSD$ */
 
 /* The latter of the two options wins. */
@@ -9,12 +9,14 @@
 -/* comment */bacc
 -T/* define a type */custom_type
 
+/* For int options, trailing garbage would lead to an error message. */
+-i3
+
 /*
- * For int or float options, trailing garbage is ignored.
+ * For float options, trailing garbage is ignored.
  *
- * See atoi, atof.
+ * See atof.
  */
--i3garbage
 -cli3.5garbage
 
 -b/*/acc       /* The comment is '/' '*' '/', making the option '-bacc'. */
diff -r 483a4aab94fa -r d9cd5e890081 tests/usr.bin/indent/t_errors.sh
--- a/tests/usr.bin/indent/t_errors.sh  Sun Oct 17 17:23:59 2021 +0000
+++ b/tests/usr.bin/indent/t_errors.sh  Sun Oct 17 18:13:00 2021 +0000
@@ -1,5 +1,5 @@
 #! /bin/sh
-# $NetBSD: t_errors.sh,v 1.3 2021/10/14 18:55:41 rillig Exp $
+# $NetBSD: t_errors.sh,v 1.4 2021/10/17 18:13:00 rillig Exp $
 #
 # Copyright (c) 2021 The NetBSD Foundation, Inc.
 # All rights reserved.
@@ -97,7 +97,7 @@
 option_tabsize_zero_body()
 {
        expect_error \
-           'indent: invalid tabsize 0' \
+           'indent: Command line: invalid argument "0" for option "-ts"' \
            -ts0
 }
 
@@ -106,7 +106,7 @@
 {
        # Integer overflow, on both ILP32 and LP64 platforms.
        expect_error \
-           'indent: invalid tabsize 81' \
+           'indent: Command line: invalid argument "81" for option "-ts"' \
            -ts81
 }
 
@@ -115,7 +115,7 @@
 {
        # Integer overflow, on both ILP32 and LP64 platforms.
        expect_error \
-           'indent: invalid tabsize -1294967296' \
+           'indent: Command line: invalid argument "3000000000" for option "-ts"' \
            -ts3000000000
 }
 
@@ -123,10 +123,18 @@
 option_indent_size_zero_body()
 {
        expect_error \
-           'indent: invalid indentation 0' \
+           'indent: Command line: invalid argument "0" for option "-i"' \
            -i0
 }
 
+atf_test_case 'option_int_trailing_garbage'
+option_int_trailing_garbage_body()
+{
+       expect_error \
+           'indent: Command line: invalid argument "3garbage" for option "-i"' \
+           -i3garbage
+}
+
 atf_test_case 'option_buffer_overflow'
 option_buffer_overflow_body()
 {
@@ -345,6 +353,7 @@
        atf_add_test_case 'option_tabsize_zero'
        atf_add_test_case 'option_tabsize_large'
        atf_add_test_case 'option_tabsize_very_large'
+       atf_add_test_case 'option_int_trailing_garbage'
        atf_add_test_case 'option_indent_size_zero'
        atf_add_test_case 'unterminated_comment'
        atf_add_test_case 'in_place_wrong_backup'
diff -r 483a4aab94fa -r d9cd5e890081 usr.bin/indent/args.c
--- a/usr.bin/indent/args.c     Sun Oct 17 17:23:59 2021 +0000
+++ b/usr.bin/indent/args.c     Sun Oct 17 18:13:00 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: args.c,v 1.55 2021/10/13 23:33:52 rillig Exp $ */
+/*     $NetBSD: args.c,v 1.56 2021/10/17 18:13:00 rillig Exp $ */
 
 /*-
  * SPDX-License-Identifier: BSD-4-Clause
@@ -43,7 +43,7 @@
 
 #include <sys/cdefs.h>
 #if defined(__NetBSD__)
-__RCSID("$NetBSD: args.c,v 1.55 2021/10/13 23:33:52 rillig Exp $");
+__RCSID("$NetBSD: args.c,v 1.56 2021/10/17 18:13:00 rillig Exp $");
 #elif defined(__FreeBSD__)
 __FBSDID("$FreeBSD: head/usr.bin/indent/args.c 336318 2018-07-15 21:04:21Z pstef $");
 #endif
@@ -56,6 +56,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <errno.h>
 
 #include "indent.h"
 
@@ -68,11 +69,11 @@
 #endif
 
 #define bool_option(name, value, var) \
-       {name, true, value, false, assert_type(&(opt.var), bool *)}
+       {name, true, value, false, 0, 0, assert_type(&(opt.var), bool *)}
 #define bool_options(name, var) \
-       {name, true, false, true, assert_type(&(opt.var), bool *)}
-#define int_option(name, var) \
-       {name, false, false, false, assert_type(&(opt.var), int *)}
+       {name, true, false, true, 0, 0, assert_type(&(opt.var), bool *)}
+#define int_option(name, var, min, max) \
+       {name, false, false, false, min, max, assert_type(&(opt.var), int *)}
 
 /*
  * N.B.: an option whose name is a prefix of another option must come earlier;
@@ -85,6 +86,8 @@
     bool p_is_bool;
     bool p_bool_value;
     bool p_may_negate;
+    short i_min;
+    short i_max;
     void *p_var;               /* the associated variable */
 }   pro[] = {
     bool_options("bacc", blanklines_around_conditional_compilation),
@@ -96,26 +99,26 @@
     bool_option("bl", false, brace_same_line),
     bool_option("br", true, brace_same_line),
     bool_options("bs", blank_after_sizeof),
-    int_option("c", comment_column),
-    int_option("cd", decl_comment_column),
+    int_option("c", comment_column, 1, 999),
+    int_option("cd", decl_comment_column, 1, 999),
     bool_options("cdb", comment_delimiter_on_blankline),
     bool_options("ce", cuddle_else),
-    int_option("ci", continuation_indent),
+    int_option("ci", continuation_indent, 0, 999),
     /* "cli" is special */
     bool_options("cs", space_after_cast),
-    int_option("d", unindent_displace),
-    int_option("di", decl_indent),
+    int_option("d", unindent_displace, -999, 999),
+    int_option("di", decl_indent, 0, 999),
     bool_options("dj", ljust_decl),
     bool_options("eei", extra_expr_indent),
     bool_options("ei", else_if),
     bool_options("fbs", function_brace_split),
     bool_options("fc1", format_col1_comments),
     bool_options("fcb", format_block_comments),
-    int_option("i", indent_size),
+    int_option("i", indent_size, 1, 80),
     bool_options("ip", indent_parameters),
-    int_option("l", max_line_length),
-    int_option("lc", block_comment_max_line_length),
-    int_option("ldi", local_decl_indent),
+    int_option("l", max_line_length, 1, 999),
+    int_option("lc", block_comment_max_line_length, 1, 999),
+    int_option("ldi", local_decl_indent, 0, 999),
     bool_options("lp", lineup_to_parens),
     bool_options("lpl", lineup_to_parens_always),
     /* "npro" is special */
@@ -127,7 +130,7 @@
     /* "st" is special */
     bool_option("ta", true, auto_typedefs),
     /* "T" is special */
-    int_option("ts", tabsize),
+    int_option("ts", tabsize, 1, 80),
     /* "U" is special */
     bool_options("ut", use_tabs),
     bool_options("v", verbose),
@@ -299,6 +302,13 @@
        if (!isdigit((unsigned char)*param_start))
            errx(1, "%s: option \"-%s\" requires an integer parameter",
                option_source, p->p_name);
-       *(int *)p->p_var = atoi(param_start);
+       errno = 0;
+       char *end;
+       long num = strtol(param_start, &end, 10);
+       if (!(errno == 0 && *end == '\0' &&
+               p->i_min <= num && num <= p->i_max))
+           errx(1, "%s: invalid argument \"%s\" for option \"-%s\"",
+                option_source, param_start, p->p_name);
+       *(int *)p->p_var = (int)num;
     }
 }
diff -r 483a4aab94fa -r d9cd5e890081 usr.bin/indent/indent.c
--- a/usr.bin/indent/indent.c   Sun Oct 17 17:23:59 2021 +0000
+++ b/usr.bin/indent/indent.c   Sun Oct 17 18:13:00 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: indent.c,v 1.137 2021/10/09 11:13:25 rillig Exp $      */
+/*     $NetBSD: indent.c,v 1.138 2021/10/17 18:13:00 rillig Exp $      */
 
 /*-
  * SPDX-License-Identifier: BSD-4-Clause
@@ -43,7 +43,7 @@
 
 #include <sys/cdefs.h>
 #if defined(__NetBSD__)
-__RCSID("$NetBSD: indent.c,v 1.137 2021/10/09 11:13:25 rillig Exp $");
+__RCSID("$NetBSD: indent.c,v 1.138 2021/10/17 18:13:00 rillig Exp $");
 #elif defined(__FreeBSD__)
 __FBSDID("$FreeBSD: head/usr.bin/indent/indent.c 340138 2018-11-04 19:24:49Z oshogbo $");
 #endif
@@ -559,10 +559,6 @@
            : opt.comment_column;
     if (opt.continuation_indent == 0)
        opt.continuation_indent = opt.indent_size;
-    if (!(1 <= opt.tabsize && opt.tabsize <= 80))
-       errx(EXIT_FAILURE, "invalid tabsize %d", opt.tabsize);
-    if (!(1 <= opt.indent_size && opt.indent_size <= 80))
-       errx(EXIT_FAILURE, "invalid indentation %d", opt.indent_size);
 }
 
 static void