[src/trunk]: src/usr.bin/sort Rework previous change to fixit() to not trip o...

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

details:   https://anonhg.NetBSD.org/src/rev/c4eb1ef73f6a
branches:  trunk
changeset: 755426:c4eb1ef73f6a
user:      dholland <dholland%NetBSD.org@localhost>
date:      Sat Jun 05 17:46:08 2010 +0000

description:
Rework previous change to fixit() to not trip on option arguments. (Noticed
by wiz.) Clarify the loop logic involved.

diffstat:

 usr.bin/sort/init.c |  121 +++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 91 insertions(+), 30 deletions(-)

diffs (181 lines):

diff -r e5cc13a843eb -r c4eb1ef73f6a usr.bin/sort/init.c
--- a/usr.bin/sort/init.c       Sat Jun 05 17:44:51 2010 +0000
+++ b/usr.bin/sort/init.c       Sat Jun 05 17:46:08 2010 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: init.c,v 1.25 2010/05/27 05:52:29 dholland Exp $       */
+/*     $NetBSD: init.c,v 1.26 2010/06/05 17:46:08 dholland Exp $       */
 
 /*-
  * Copyright (c) 2000-2003 The NetBSD Foundation, Inc.
@@ -63,7 +63,7 @@
 
 #include "sort.h"
 
-__RCSID("$NetBSD: init.c,v 1.25 2010/05/27 05:52:29 dholland Exp $");
+__RCSID("$NetBSD: init.c,v 1.26 2010/06/05 17:46:08 dholland Exp $");
 
 #include <ctype.h>
 #include <string.h>
@@ -238,38 +238,99 @@
 }
 
 /*
+ * Return true if the options found in ARG, according to the getopt
+ * spec in OPTS, require an additional argv word as an option
+ * argument.
+ */
+static int
+options_need_argument(const char *arg, const char *opts)
+{
+       size_t pos;
+       const char *s;
+
+       /*assert(arg[0] == '-');*/
+
+       pos = 1;
+       while (arg[pos]) {
+               s = strchr(opts, arg[pos]);
+               if (s == NULL) {
+                       /* invalid option */
+                       return 0;
+               }
+               if (s[1] == ':') {
+                       /* option requires argument */
+                       if (arg[pos+1] == '\0') {
+                               /* no argument in this arg */
+                               return 1;
+                       }
+                       else {
+                               /* argument is in this arg; no more options */
+                               return 0;
+                       }
+               }
+               pos++;
+       }
+       return 0;
+}
+
+/*
  * Replace historic +SPEC arguments with appropriate -kSPEC.
+ *
+ * The form can be either a single +SPEC or a pair +SPEC -SPEC.
+ * The followiing -SPEC is not recognized unless it follows
+ * immediately.
  */ 
 void
-fixit(int *argc, char **argv)
+fixit(int *argc, char **argv, const char *opts)
 {
-       int i, j, fplus=0;
+       int i, j, sawplus;
        char *vpos, *tpos, spec[20];
        int col, indent;
        size_t sz;
 
+       sawplus = 0;
        for (i = 1; i < *argc; i++) {
                /*
-                * Stop where getopt will stop, to avoid turning e.g.
-                * "sort x +3" into "sort x -k4.1" which will croak if
-                * +3 was in fact really a file name.
+                * This loop must stop exactly where getopt will stop.
+                * Otherwise it turns e.g. "sort x +3" into "sort x
+                * -k4.1", which will croak if +3 was in fact really a
+                * file name. In order to do this reliably we need to
+                * be able to identify argv words that are option
+                * arguments.
                 */
-               if (argv[i][0] == '-' && argv[i][1] == '-')
-                       break;
-               if (argv[i][0] != '-' && argv[i][0] != '+')
-                       break;
 
-               if (argv[i][0] != '+' && !fplus)
-                       continue;
-
-               if (fplus && (argv[i][0] != '-' || !isdigit((unsigned char)argv[i][1]))) {
-                       fplus = 0;
-                       if (argv[i][0] != '+') {
-                               /* not a -POS argument, skip */
-                               continue;
-                       }
+               if (!strcmp(argv[i], "--")) {
+                       /* End of options; stop. */
+                       break;
                }
 
+               if (argv[i][0] == '+') {
+                       /* +POS argument */
+                       sawplus = 1;
+               } else if (argv[i][0] == '-' && sawplus &&
+                          isdigit((unsigned char)argv[i][1])) {
+                       /* -POS argument */
+                       sawplus = 0;
+               } else if (argv[i][0] == '-') {
+                       /* other option */
+                       sawplus = 0;
+                       if (options_need_argument(argv[i], opts)) {
+                               /* skip over the argument */
+                               i++;
+                       }
+                       continue;
+               } else {
+                       /* not an option at all; stop */
+                       sawplus = 0;
+                       break;
+               }
+
+               /*
+                * At this point argv[i] is an old-style spec. The
+                * sawplus flag used by the above loop logic also
+                * tells us if it's a +SPEC or -SPEC.
+                */
+               
                /* parse spec */
                tpos = argv[i]+1;
                col = (int)strtol(tpos, &tpos, 10);
@@ -278,14 +339,14 @@
                        indent = (int) strtol(tpos, &tpos, 10);
                } else
                        indent = 0;
-               /* tpos points to optional flags now */
+               /* tpos now points to the optional flags */
 
                /*
-                * For x.y, the obsolescent variant assumed 0 == beginning
-                * of line, while the new form uses 0 == end of line.
-                * Convert accordingly.
+                * In the traditional form, x.0 means beginning of line;
+                * in the new form, x.0 means end of line. Adjust the
+                * value of INDENT accordingly.
                 */
-               if (!fplus) {
+               if (sawplus) {
                        /* +POS */
                        col += 1;
                        indent += 1;
@@ -295,19 +356,19 @@
                                col += 1;
                }
 
-               /* new style spec */
+               /* make the new style spec */
                sz = snprintf(spec, sizeof(spec), "%d.%d%s", col, indent,
                    tpos);
 
-               if (!fplus) {
+               if (sawplus) {
                        /* Replace the +POS argument with new-style -kSPEC */
                        asprintf(&vpos, "-k%s", spec);
                        argv[i] = vpos;
-                       fplus = 1;
                } else {
                        /*
-                        * Append the spec to one previously generated from
-                        * +POS argument, and remove the argv element.
+                        * Append the spec to the one from the
+                        * preceding +POS argument, and remove the
+                        * current argv element entirely.
                         */
                        asprintf(&vpos, "%s,%s", argv[i-1], spec);
                        free(argv[i-1]);