On 02/16/2012 09:30 PM, Jérémy Compostella wrote:
> Pádraig, all,
> 
> I rebased my branch for this feature and make the syntax-check
> success. I attached the new patch which I hope will satisfy you.
> 
> Feel free to comment it, I will take into account whatever you want.

Thanks for continuing with this.
One general thing that might both improve
and simplify the implementation, is to
not to convert from string to int at all.

I.E. when processing the arg, just validate like:
if (strlen (optarg) != strspn (optarg, suffix_alphabet))
  error()
else
   /* skip over any leading 0, and use this as the start directly. */

Then the subsequent check for length and
the initialization of the file name should be simplified.

Also this removes the limitation of size of an unsigned int,
though that's not really a practical concern I suppose.

I've also attached some string and test cleanups,
to --amend into your patch.

cheers,
Pádraig.
diff --git a/NEWS b/NEWS
index fae6ecc..4662919 100644
--- a/NEWS
+++ b/NEWS
@@ -7,10 +7,8 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   dd now accepts the count_bytes, skip_bytes iflags and the count_bytes
   oflag, to more easily allow processing portions of a file.
 
-  split now accept an optional "from" value for the
-  --numeric-suffixes option. If this argument is specified, the
-  numeric suffix counts from this value, otherwise, like before, it
-  counts from 0.
+  split now accepts an optional "from" argument to --numeric-suffixes,
+  which changes the start number from the default of 0.
 
 ** Bug fixes
 
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index f5616d8..d7a43d6 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -3087,7 +3087,7 @@ Use suffixes of length @var{length}.  The default 
@var{length} is 2.
 @itemx --numeric-suffixes[=@var{from}]
 @opindex -d
 @opindex --numeric-suffixes
-Use digits in suffixes rather than lower-case letters. The numerical
+Use digits in suffixes rather than lower-case letters.  The numerical
 suffix counts from @var{from} if specified, 0 otherwise.
 
 @item -e
diff --git a/src/split.c b/src/split.c
index 03feb9f..e7c61bb 100644
--- a/src/split.c
+++ b/src/split.c
@@ -249,8 +249,7 @@ next_file_name (void)
       outfile_mid = outfile + outbase_length;
       memcpy (outfile, outbase, outbase_length);
       sufindex = xcalloc (suffix_length, sizeof *sufindex);
-      /* Initialize the suffix index accordingly to the count from
-         value.  */
+      /* Initialize the suffix index according to start value.  */
       {
         unsigned long left = suffix_count_from;
         while (i-- != 0)
@@ -1166,7 +1165,7 @@ main (int argc, char **argv)
               if (xstrtoul (optarg, NULL, 10, &tmp, "") != LONGINT_OK)
                 {
                   error (0, 0,
-                         _("%s: invalid count from numerical suffix number"),
+                         _("%s: invalid start value for numerical suffix"),
                          optarg);
                   usage (EXIT_FAILURE);
                 }
@@ -1243,7 +1242,7 @@ main (int argc, char **argv)
       usage (EXIT_FAILURE);
     }
 
-  /* Check that the suffix length is greater enough for the numerical
+  /* Check that the suffix length is large enough for the numerical
      suffix count from value.  */
   if (suffix_count_from)
     {
@@ -1254,8 +1253,8 @@ main (int argc, char **argv)
         {
           if (length == 0)
             {
-              error (0, 0, _("numerical suffix FROM number too hight\
- for the suffix length"));
+              error (0, 0, _("numerical suffix start value is too large "
+                             "for the suffix length"));
               usage (EXIT_FAILURE);
             }
           start /= 10;
diff --git a/tests/split/numeric b/tests/split/numeric
index 5550d6f..ad22df6 100755
--- a/tests/split/numeric
+++ b/tests/split/numeric
@@ -21,7 +21,7 @@ print_ver_ split
 
 # Check default start from 0
 printf '1\n2\n3\n4\n5\n' > in || framework_failure_
-split --numeric-suffixes --lines=2 in > out || fail=1
+split --numeric-suffixes --lines=2 in || fail=1
 cat <<\EOF > exp-1
 1
 2
@@ -38,7 +38,7 @@ compare exp-2 x01 || fail=1
 compare exp-3 x02 || fail=1
 
 # Check --numeric-suffixes=X
-split --numeric-suffixes=1 --lines=2 in > out || fail=1
+split --numeric-suffixes=1 --lines=2 in || fail=1
 cat <<\EOF > exp-1
 1
 2
@@ -54,8 +54,12 @@ compare exp-1 x01 || fail=1
 compare exp-2 x02 || fail=1
 compare exp-3 x03 || fail=1
 
-# Check that split failed when suffix length is not greater enough for
-# the numerical suffix count from value
-fail=1 && split -a 3 --numeric-suffixes=1000 in > out 2> /dev/null || fail=0
+# Check that split failed when suffix length is not large enough for
+# the numerical suffix start value
+split -a 3 --numeric-suffixes=1000 in 2> /dev/null && fail=1
+
+# check invalid --numeric-suffixes start values are flagged
+split --numeric-suffixes=-1 in 2> /dev/null && fail=1
+split --numeric-suffixes=one in 2> /dev/null && fail=1
 
 Exit $fail

Reply via email to