[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Helpful diagnostic for "cut -f0" and similar
|
From: |
Jim Meyering |
|
Subject: |
Re: Helpful diagnostic for "cut -f0" and similar |
|
Date: |
Tue, 22 May 2007 18:48:59 +0200 |
"James Youngman" <address@hidden> wrote:
> 2007-05-22 James Youngman <address@hidden>
>
> * src/cut.c (sanity_check_pos): check arguments to -f and -c
> to better diagnose possible use of column 0, which is
> invalid. These were previously being rejected, correctly
> so, but with a less than helpful error message.
> (ADD_RANGE_PAIR): call sanity_check_pos
> (set_fields): remove a special case for value==0, because
> ADD_RANGE_PAIR rejects it anyway.
Hi James
Thank you!
In looking at that, I spotted a couple of other cases.
Here's my current state:
2007-05-22 Jim Meyering <address@hidden>
cut: diagnose a range starting with 0 (-f 0-2) as invalid, and
give a better diagnostic for a field-number/offset of 0.
* NEWS: Mention the fix.
* src/cut.c (ADD_RANGE_PAIR): Add an explicit check.
Based on a patch from James Youngman.
* tests/misc/cut: Add tests for the above.
"cut -f 2-0" now fails; before, it was equivalent to "cut -f 2-"
Also, diagnose the '-' in "cut -f -" as an invalid range, rather
than interpreting it as the unlimited range, "1-".
* NEWS: Mention these changes.
* src/cut.c (set_fields): Don't interpret an accumulator "value"
of 0 as an unspecified range endpoint.
Give better diagnostics.
Adjust a comment so that it is true also for 64-bit size_t.
* tests/cut/Test.pm: Add tests for the above.
Signed-off-by: Jim Meyering <address@hidden>
---
ChangeLog | 10 ++++++++++
NEWS | 5 +++++
src/cut.c | 31 +++++++++++++++++++------------
tests/cut/Test.pm | 9 ++++++++-
4 files changed, 42 insertions(+), 13 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index b7a504b..c1fe872 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
2007-05-22 Jim Meyering <address@hidden>
+ "cut -f 2-0" now fails; before, it was equivalent to "cut -f 2-"
+ Also, diagnose the '-' in "cut -f -" as an invalid range, rather
+ than interpreting it as the unlimited range, "1-".
+ * NEWS: Mention these changes.
+ * src/cut.c (set_fields): Don't interpret an accumulator "value"
+ of 0 as an unspecified range endpoint.
+ Give better diagnostics.
+ Adjust a comment so that it is true also for 64-bit size_t.
+ * tests/cut/Test.pm: Add tests for the above.
+
stty: fix a harmless syntax nit
* src/stty.c (visible): Use ";" as the statement terminator
between two assignments, not ",".
diff --git a/NEWS b/NEWS
index 0000a2b..0326681 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,11 @@ GNU coreutils NEWS -*-
outline -*-
** Bug fixes
+ "cut -f 2-0" now fails; before, it was equivalent to "cut -f 2-"
+
+ cut now diagnoses the '-' in "cut -f -" as an invalid range, rather
+ than interpreting it as the unlimited range, "1-".
+
ls -x DIR would sometimes output the wrong string in place of the
first entry. [introduced in coreutils-6.8]
diff --git a/src/cut.c b/src/cut.c
index 0883e72..ab14abc 100644
--- a/src/cut.c
+++ b/src/cut.c
@@ -342,6 +342,8 @@ set_fields (const char *fieldstr)
{
size_t initial = 1; /* Value of first number in a range. */
size_t value = 0; /* If nonzero, a number being accumulated. */
+ bool lhs_specified = false;
+ bool rhs_specified = false;
bool dash_found = false; /* True if a '-' is found in this field. */
bool field_found = false; /* True if at least one field spec
has been processed. */
@@ -366,13 +368,8 @@ set_fields (const char *fieldstr)
dash_found = true;
fieldstr++;
- if (value)
- {
- initial = value;
- value = 0;
- }
- else
- initial = 1;
+ initial = (lhs_specified ? value : 1);
+ value = 0;
}
else if (*fieldstr == ',' || isblank (*fieldstr) || *fieldstr == '\0')
{
@@ -382,9 +379,12 @@ set_fields (const char *fieldstr)
{
dash_found = false;
- /* A range. Possibilites: -n, m-n, n-.
+ if (!lhs_specified && !rhs_specified)
+ FATAL_ERROR (_("invalid range with no endpoint: -"));
+
+ /* A range. Possibilities: -n, m-n, n-.
In any case, `initial' contains the start of the range. */
- if (value == 0)
+ if (!rhs_specified)
{
/* `n-'. From `initial' to end of line. */
eol_range_start = initial;
@@ -394,7 +394,7 @@ set_fields (const char *fieldstr)
{
/* `m-n' or `-n' (1-n). */
if (value < initial)
- FATAL_ERROR (_("invalid byte or field list"));
+ FATAL_ERROR (_("invalid decreasing range"));
/* Is there already a range going to end of line? */
if (eol_range_start != 0)
@@ -432,7 +432,7 @@ set_fields (const char *fieldstr)
value = 0;
}
}
- else if (value != 0)
+ else
{
/* A simple field number, not a range. */
ADD_RANGE_PAIR (rp, value, value);
@@ -446,6 +446,8 @@ set_fields (const char *fieldstr)
}
fieldstr++;
+ lhs_specified = false;
+ rhs_specified = false;
}
else if (ISDIGIT (*fieldstr))
{
@@ -456,10 +458,15 @@ set_fields (const char *fieldstr)
num_start = fieldstr;
in_digits = true;
+ if (dash_found)
+ rhs_specified = 1;
+ else
+ lhs_specified = 1;
+
/* Detect overflow. */
if (!DECIMAL_DIGIT_ACCUMULATE (value, *fieldstr - '0', size_t))
{
- /* In case the user specified -c4294967296,22,
+ /* In case the user specified -c$(echo 2^64|bc),22,
complain only about the first number. */
/* Determine the length of the offending number. */
size_t len = strspn (num_start, "0123456789");
diff --git a/tests/cut/Test.pm b/tests/cut/Test.pm
index b4cd4e5..109a13f 100755
--- a/tests/cut/Test.pm
+++ b/tests/cut/Test.pm
@@ -1,6 +1,6 @@
# Test 'cut'.
-# Copyright (C) 1996, 1997, 1998, 1999, 2003, 2004 Free Software
+# Copyright (C) 1996, 1997, 1998, 1999, 2003, 2004, 2007 Free Software
# Foundation, Inc.
# This program is free software; you can redistribute it and/or modify
@@ -113,6 +113,13 @@ my @tv = (
['od-overlap4', '-b1-3,2-3 --output-d=:', "abcd\n", "abc\n", 0],
['od-overlap5', '-b1-3,1-4 --output-d=:', "abcde\n", "abcd\n", 0],
+# None of the following invalid ranges provoked an error before coreutils-6.10.
+['inval1', '-f 2-0', '', '', 1],
+['inval2', '-f -', '', '', 1],
+['inval3', '-f 4,-', '', '', 1],
+['inval4', '-f 1-2,-', '', '', 1],
+['inval5', '-f 1-,-', '', '', 1],
+['inval6', '-f -1,-', '', '', 1],
);
# Don't use a pipe for failing tests. Otherwise, sometimes they
Signed-off-by: Jim Meyering <address@hidden>
---
ChangeLog | 8 ++++++++
NEWS | 3 +++
src/cut.c | 2 ++
tests/misc/cut | 25 +++++++++++++++++++------
4 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index c1fe872..4642667 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
2007-05-22 Jim Meyering <address@hidden>
+ cut: diagnose a range starting with 0 (-f 0-2) as invalid, and
+ give a better diagnostic for a field-number/offset of 0.
+ * NEWS: Mention the fix.
+ * src/cut.c (ADD_RANGE_PAIR): Add an explicit check.
+ Based on a patch from James Youngman.
+ * tests/misc/cut: Add tests for the above.
+
"cut -f 2-0" now fails; before, it was equivalent to "cut -f 2-"
Also, diagnose the '-' in "cut -f -" as an invalid range, rather
than interpreting it as the unlimited range, "1-".
@@ -8,6 +15,7 @@
of 0 as an unspecified range endpoint.
Give better diagnostics.
Adjust a comment so that it is true also for 64-bit size_t.
+
* tests/cut/Test.pm: Add tests for the above.
stty: fix a harmless syntax nit
diff --git a/NEWS b/NEWS
index 0326681..ea08e0a 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,9 @@ GNU coreutils NEWS -*-
outline -*-
** Bug fixes
+ cut now diagnoses a range starting with zero (e.g., -f 0-2) as invalid;
+ before, it would treat it as if it started with 1 (-f 1-2).
+
"cut -f 2-0" now fails; before, it was equivalent to "cut -f 2-"
cut now diagnoses the '-' in "cut -f -" as an invalid range, rather
diff --git a/src/cut.c b/src/cut.c
index ab14abc..e89460e 100644
--- a/src/cut.c
+++ b/src/cut.c
@@ -57,6 +57,8 @@
#define ADD_RANGE_PAIR(rp, low, high) \
do \
{ \
+ if (low == 0 || high == 0) \
+ FATAL_ERROR (_("fields and positions are numbered from 1")); \
if (n_rp >= n_rp_allocated) \
{ \
(rp) = X2NREALLOC (rp, &n_rp_allocated); \
diff --git a/tests/misc/cut b/tests/misc/cut
index 3db4c9b..803fbc1 100755
--- a/tests/misc/cut
+++ b/tests/misc/cut
@@ -1,7 +1,7 @@
#!/bin/sh
# Test "cut". -*- perl -*-
-# Copyright (C) 2006 Free Software Foundation, Inc.
+# Copyright (C) 2006, 2007 Free Software Foundation, Inc.
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -27,7 +27,7 @@ $PERL -e 1 > /dev/null 2>&1 || {
exit 77
}
-exec $PERL -w -I$srcdir/.. -MCoreutils -- - <<\EOF
+exec $PERL -w -I$srcdir/.. -MCoreutils -- - <<\FILE_EOF
require 5.003;
use strict;
@@ -36,16 +36,29 @@ use strict;
# Turn off localisation of executable's ouput.
@ENV{qw(LANGUAGE LANG LC_ALL)} = ('C') x 3;
+my $prog = 'cut';
+my $diag = <<EOF;
+$prog: fields and positions are numbered from 1
+Try \`cut --help' for more information.
+EOF
+
my @Tests =
(
- # Provoke a double-free in cut from coreutils-6.7.
- ['dbl-free', '-f2-', {IN=>{f=>'x'}}, {IN=>{g=>'y'}}, {OUT=>"x\ny\n"}],
+ # Provoke a double-free in cut from coreutils-6.7.
+ ['dbl-free', '-f2-', {IN=>{f=>'x'}}, {IN=>{g=>'y'}}, {OUT=>"x\ny\n"}],
+
+ # This failed (as it should) even before coreutils-6.10,
+ # but cut from 6.10 produces a more useful diagnostic.
+ ['zero-1', '-b0', {ERR=>$diag}, {EXIT => 1} ],
+
+ # Before coreutils-6.10, specifying a range of 0-2 was not an error.
+ # It was treated just like "-2".
+ ['zero-2', '-f0-2', {ERR=>$diag}, {EXIT => 1} ],
);
my $save_temps = $ENV{DEBUG};
my $verbose = $ENV{VERBOSE};
-my $prog = 'cut';
my $fail = run_tests ($ME, $prog, address@hidden, $save_temps, $verbose);
exit $fail;
-EOF
+FILE_EOF