Re: [PATCH] cut: improve detection of invalid ranges

2024-12-09 Thread Denys Vlasenko
Applied, thank you!

On Sun, Nov 3, 2024 at 1:47 PM Ron Yorston  wrote:
>
> Commit 0068ce2fa (cut: add toybox-compatible options -O OUTSEP,
> -D, -F LIST) added detection of reversed ranges.  Further
> improvements are possible.
>
> - The test for reversed ranges compared the start after it had been
>   decremented with the end before decrement.  It thus missed ranges
>   of the form 2-1.
>
> - Zero isn't a valid start value for a range.  (Nor is it a valid
>   end value, but that's caught by the test for a reversed range.)
>
> - The code
>
> if (!*ltok)
> e = INT_MAX;
>
>   duplicates a check that's already been made.
>
> - Display the actual range in the error message to make it easier
>   to find which range was at fault.
>
> function old new   delta
> .rodata   100273  100287 +14
> cut_main12391237  -2
> --
> (add/remove: 0/0 grow/shrink: 1/1 up/down: 14/-2)  Total: 12 bytes
>
> Signed-off-by: Ron Yorston 
> ---
>  coreutils/cut.c | 19 ---
>  testsuite/cut.tests |  5 -
>  2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/coreutils/cut.c b/coreutils/cut.c
> index d129f9b9d..6717122c5 100644
> --- a/coreutils/cut.c
> +++ b/coreutils/cut.c
> @@ -268,29 +268,26 @@ int cut_main(int argc UNUSED_PARAM, char **argv)
> if (!ntok[0]) {
> s = 0;
> } else {
> -   s = xatoi_positive(ntok);
> /* account for the fact that arrays are zero 
> based, while
>  * the user expects the first char on the 
> line to be char #1 */
> -   if (s != 0)
> -   s--;
> +   s = xatoi_positive(ntok) - 1;
> }
>
> /* get the end pos */
> if (ltok == NULL) {
> e = s;
> } else if (!ltok[0]) {
> +   /* if the user specified no end position,
> +* that means "til the end of the line" */
> e = INT_MAX;
> } else {
> -   e = xatoi_positive(ltok);
> -   /* if the user specified and end position of 
> 0,
> -* that means "til the end of the line" */
> -   if (!*ltok)
> -   e = INT_MAX;
> -   else if (e < s)
> -   bb_error_msg_and_die("%d<%d", e, s);
> -   e--;/* again, arrays are zero based, 
> lines are 1 based */
> +   /* again, arrays are zero based, lines are 1 
> based */
> +   e = xatoi_positive(ltok) - 1;
> }
>
> +   if (s < 0 || e < s)
> +   bb_error_msg_and_die("invalid range %s-%s", 
> ntok, ltok ?: ntok);
> +
> /* add the new list */
> cut_lists = xrealloc_vector(cut_lists, 4, nlists);
> /* NB: startpos is always >= 0 */
> diff --git a/testsuite/cut.tests b/testsuite/cut.tests
> index 2458c019c..a31f41f7f 100755
> --- a/testsuite/cut.tests
> +++ b/testsuite/cut.tests
> @@ -31,7 +31,10 @@ testing "-b encapsulated" "cut -b 3-8,4-6 input" 
> "e:two:\npha:be\ne quic\n" \
>  #testing "cut -bO overlaps" \
>  #  "cut --output-delimiter ' ' -b 1-3,2-5,7-9,9-10 input" \
>  #  "one:t o:th\nalpha beta\nthe q ick \n" "$abc" ""
> -testing "cut high-low error" "cut -b 8-3 abc.txt 2>/dev/null || echo err" 
> "err\n" \
> +testing "cut high-low error" "cut -b 8-3 input 2>/dev/null || echo err" 
> "err\n" \
> +  "$abc" ""
> +
> +testing "cut -b 2-1 error" "cut -b 2-1 input 2>/dev/null || echo err" 
> "err\n" \
>"$abc" ""
>
>  testing "cut -c a-b" "cut -c 4-10 input" ":two:th\nha:beta\n quick \n" 
> "$abc" ""
> --
> 2.47.0
>
> ___
> busybox mailing list
> busybox@busybox.net
> https://lists.busybox.net/mailman/listinfo/busybox
___
busybox mailing list
busybox@busybox.net
https://lists.busybox.net/mailman/listinfo/busybox


[PATCH] cut: improve detection of invalid ranges

2024-11-03 Thread Ron Yorston
Commit 0068ce2fa (cut: add toybox-compatible options -O OUTSEP,
-D, -F LIST) added detection of reversed ranges.  Further
improvements are possible.

- The test for reversed ranges compared the start after it had been
  decremented with the end before decrement.  It thus missed ranges
  of the form 2-1.

- Zero isn't a valid start value for a range.  (Nor is it a valid
  end value, but that's caught by the test for a reversed range.)

- The code

if (!*ltok)
e = INT_MAX;

  duplicates a check that's already been made.

- Display the actual range in the error message to make it easier
  to find which range was at fault.

function old new   delta
.rodata   100273  100287 +14
cut_main12391237  -2
--
(add/remove: 0/0 grow/shrink: 1/1 up/down: 14/-2)  Total: 12 bytes

Signed-off-by: Ron Yorston 
---
 coreutils/cut.c | 19 ---
 testsuite/cut.tests |  5 -
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/coreutils/cut.c b/coreutils/cut.c
index d129f9b9d..6717122c5 100644
--- a/coreutils/cut.c
+++ b/coreutils/cut.c
@@ -268,29 +268,26 @@ int cut_main(int argc UNUSED_PARAM, char **argv)
if (!ntok[0]) {
s = 0;
} else {
-   s = xatoi_positive(ntok);
/* account for the fact that arrays are zero 
based, while
 * the user expects the first char on the line 
to be char #1 */
-   if (s != 0)
-   s--;
+   s = xatoi_positive(ntok) - 1;
}
 
/* get the end pos */
if (ltok == NULL) {
e = s;
} else if (!ltok[0]) {
+   /* if the user specified no end position,
+* that means "til the end of the line" */
e = INT_MAX;
} else {
-   e = xatoi_positive(ltok);
-   /* if the user specified and end position of 0,
-* that means "til the end of the line" */
-   if (!*ltok)
-   e = INT_MAX;
-   else if (e < s)
-   bb_error_msg_and_die("%d<%d", e, s);
-   e--;/* again, arrays are zero based, lines 
are 1 based */
+   /* again, arrays are zero based, lines are 1 
based */
+   e = xatoi_positive(ltok) - 1;
}
 
+   if (s < 0 || e < s)
+   bb_error_msg_and_die("invalid range %s-%s", 
ntok, ltok ?: ntok);
+
/* add the new list */
cut_lists = xrealloc_vector(cut_lists, 4, nlists);
/* NB: startpos is always >= 0 */
diff --git a/testsuite/cut.tests b/testsuite/cut.tests
index 2458c019c..a31f41f7f 100755
--- a/testsuite/cut.tests
+++ b/testsuite/cut.tests
@@ -31,7 +31,10 @@ testing "-b encapsulated" "cut -b 3-8,4-6 input" 
"e:two:\npha:be\ne quic\n" \
 #testing "cut -bO overlaps" \
 #  "cut --output-delimiter ' ' -b 1-3,2-5,7-9,9-10 input" \
 #  "one:t o:th\nalpha beta\nthe q ick \n" "$abc" ""
-testing "cut high-low error" "cut -b 8-3 abc.txt 2>/dev/null || echo err" 
"err\n" \
+testing "cut high-low error" "cut -b 8-3 input 2>/dev/null || echo err" 
"err\n" \
+  "$abc" ""
+
+testing "cut -b 2-1 error" "cut -b 2-1 input 2>/dev/null || echo err" "err\n" \
   "$abc" ""
 
 testing "cut -c a-b" "cut -c 4-10 input" ":two:th\nha:beta\n quick \n" "$abc" 
""
-- 
2.47.0

___
busybox mailing list
busybox@busybox.net
https://lists.busybox.net/mailman/listinfo/busybox