modechange improvements to catch "chmod +1", etc.

[Paul Eggert]

I installed this as a minor simplification to modechange.  The only
change visible to coreutils users is that a few invalid usages like
"chmod +1 file" and "chmod ' 1' file" are now caught.

2005-05-01  Paul Eggert  <address@hidden>

        * NEWS: "chmod +1 file" is now diagnosed.
        * lib/modechange.h (mode_free): Remove; all callers changed to invoke
        'free'.
        * lib/modechange.c: Likewise.
        xstrtol.h, stdbool.h, stddef.h: Don't include; no longer needed.
        (MODE_DONE): New constant.
        (struct mode_change): Remove 'next' member.
        (make_node_op_equals): New function; like the old one of the
        same name, except it allocates an array.
        (mode_compile, mode_create_from_ref): Use it.
        (mode_compile): Allocate result as an array, not a linked list.
        Parse octal string ourself, so that we catch mistakes like "+0".
        (mode_adjust): Arg is an array, not a linked list.

--- NEWS.~1.282.~       2005-04-29 14:00:30 -0700
+++ NEWS        2005-05-01 07:31:34 -0700
@@ -94,7 +94,7 @@ GNU coreutils NEWS                      
 
   chmod, mkdir, mkfifo, and mknod formerly mishandled rarely-used symbolic
   permissions like =xX and =u, and did not properly diagnose some invalid
-  strings like g+gr and ug,+x.  These bugs have been fixed.
+  strings like g+gr, ug,+x, and +1.  These bugs have been fixed.
 
   dd now computes statistics using a realtime clock (if available)
   rather than the time-of-day clock, to avoid glitches if the
Index: lib/modechange.c
===================================================================
RCS file: /fetish/cu/lib/modechange.c,v
retrieving revision 1.29
diff -p -u -r1.29 modechange.c
--- lib/modechange.c    28 Apr 2005 16:29:00 -0000      1.29
+++ lib/modechange.c    1 May 2005 14:25:48 -0000
@@ -19,7 +19,7 @@
 
 /* Written by David MacKenzie <address@hidden> */
 
-/* The ASCII mode string is compiled into a linked list of `struct
+/* The ASCII mode string is compiled into an array of `struct
    modechange', which can then be applied to each file to be changed.
    We do this instead of re-parsing the ASCII string for each file
    because the compiled form requires less computation to use; when
@@ -34,9 +34,6 @@
 #include <sys/stat.h>
 #include "stat-macros.h"
 #include "xalloc.h"
-#include "xstrtol.h"
-#include <stdbool.h>
-#include <stddef.h>
 #include <stdlib.h>
 
 /* The traditional octal values corresponding to each mode bit.  */
@@ -57,6 +54,9 @@
 /* Special operations flags.  */
 enum
   {
+    /* For the sentinel at the end of the mode changes array.  */
+    MODE_DONE,
+
     /* The typical case.  */
     MODE_ORDINARY_CHANGE,
 
@@ -78,10 +78,24 @@ struct mode_change
   char flag;                   /* Special operations flag.  */
   mode_t affected;             /* Set for u, g, o, or a.  */
   mode_t value;                        /* Bits to add/remove.  */
-  struct mode_change *next;    /* Link to next change in list.  */
 };
 
-/* Return a linked list of file mode change operations created from
+/* Return a mode_change array with the specified `=ddd'-style
+   mode change operation, where NEW_MODE is `ddd'.  */
+
+static struct mode_change *
+make_node_op_equals (mode_t new_mode)
+{
+  struct mode_change *p = xmalloc (2 * sizeof *p);
+  p->op = '=';
+  p->flag = MODE_ORDINARY_CHANGE;
+  p->affected = CHMOD_MODE_BITS;
+  p->value = new_mode;
+  p[1].flag = MODE_DONE;
+  return p;
+}
+
+/* Return a pointer to an array of file mode change operations created from
    MODE_STRING, an ASCII string that contains either an octal number
    specifying an absolute mode, or symbolic mode change operations with
    the form:
@@ -93,15 +107,22 @@ struct mode_change
 struct mode_change *
 mode_compile (char const *mode_string)
 {
-  struct mode_change *head;    /* First element of the linked list.  */
-  struct mode_change **tail = &head;  /* Pointer to list end.  */
-  unsigned long octal_value;   /* The mode value, if octal.  */
+  /* The array of mode-change directives to be returned.  */
+  struct mode_change *mc;
+  size_t used = 0;
 
-  if (xstrtoul (mode_string, NULL, 8, &octal_value, "") == LONGINT_OK)
+  if ('0' <= *mode_string && *mode_string < '8')
     {
       mode_t mode;
-      if (octal_value != (octal_value & ALLM))
-       return NULL;
+      unsigned int octal_value = 0;
+
+      do
+       {
+         octal_value = 8 * octal_value + *mode_string++ - '0';
+         if (ALLM < octal_value)
+           return NULL;
+       }
+      while ('0' <= *mode_string && *mode_string < '8');
 
       /* Help the compiler optimize the usual case where mode_t uses
         the traditional octal representation.  */
@@ -123,15 +144,18 @@ mode_compile (char const *mode_string)
                          | (octal_value & WOTH ? S_IWOTH : 0)
                          | (octal_value & XOTH ? S_IXOTH : 0)));
 
-      head = xmalloc (sizeof *head);
-      head->op = '=';
-      head->flag = MODE_ORDINARY_CHANGE;
-      head->affected = CHMOD_MODE_BITS;
-      head->value = mode;
-      head->next = NULL;
-      return head;
+      return make_node_op_equals (mode);
     }
 
+  /* Allocate enough space to hold the result.  */
+  {
+    size_t needed = 1;
+    char const *p;
+    for (p = mode_string; *p; p++)
+      needed += (*p == '=' || *p == '+' || *p == '-');
+    mc = xnmalloc (needed, sizeof *mc);
+  }
+
   /* One loop iteration for each `[ugoa]*([-+=]([rwxXst]*|[ugo]))+'.  */
   for (;; mode_string++)
     {
@@ -142,6 +166,8 @@ mode_compile (char const *mode_string)
       for (;; mode_string++)
        switch (*mode_string)
          {
+         default:
+           goto invalid;
          case 'u':
            affected |= S_ISUID | S_IRWXU;
            break;
@@ -156,8 +182,6 @@ mode_compile (char const *mode_string)
            break;
          case '=': case '+': case '-':
            goto no_more_affected;
-         default:
-           goto invalid;
          }
     no_more_affected:;
 
@@ -219,14 +243,11 @@ mode_compile (char const *mode_string)
            no_more_values:;
            }
 
-         change = xmalloc (sizeof *change);
+         change = &mc[used++];
          change->op = op;
          change->flag = flag;
          change->affected = affected;
          change->value = value;
-
-         *tail = change;
-         tail = &change->next;
        }
       while (*mode_string == '=' || *mode_string == '+'
             || *mode_string == '-');
@@ -237,13 +258,12 @@ mode_compile (char const *mode_string)
 
   if (*mode_string == 0)
     {
-      *tail = NULL;
-      return head;
+      mc[used].flag = MODE_DONE;
+      return mc;
     }
 
 invalid:
-  *tail = NULL;
-  mode_free (head);
+  free (mc);
   return NULL;
 }
 
@@ -253,20 +273,11 @@ invalid:
 struct mode_change *
 mode_create_from_ref (const char *ref_file)
 {
-  struct mode_change *change;  /* the only change element */
   struct stat ref_stats;
 
   if (stat (ref_file, &ref_stats) != 0)
     return NULL;
-
-  change = xmalloc (sizeof *change);
-  change->op = '=';
-  change->flag = MODE_ORDINARY_CHANGE;
-  change->affected = CHMOD_MODE_BITS;
-  change->value = ref_stats.st_mode;
-  change->next = NULL;
-
-  return change;
+  return make_node_op_equals (ref_stats.st_mode);
 }
 
 /* Return file mode OLDMODE, adjusted as indicated by the list of change
@@ -282,7 +293,7 @@ mode_adjust (mode_t oldmode, struct mode
   /* The adjusted mode.  */
   mode_t newmode = oldmode & CHMOD_MODE_BITS;
 
-  for (; changes; changes = changes->next)
+  for (; changes->flag != MODE_DONE; changes++)
     {
       mode_t affected = changes->affected;
       mode_t value = changes->value;
@@ -337,17 +348,3 @@ mode_adjust (mode_t oldmode, struct mode
 
   return newmode;
 }
-
-/* Free the memory used by the list of file mode change operations
-   CHANGES.  */
-
-void
-mode_free (struct mode_change *changes)
-{
-  while (changes)
-    {
-      struct mode_change *next = changes->next;
-      free (changes);
-      changes = next;
-    }
-}
Index: lib/modechange.h
===================================================================
RCS file: /fetish/cu/lib/modechange.h,v
retrieving revision 1.15
diff -p -u -r1.15 modechange.h
--- lib/modechange.h    28 Apr 2005 16:29:22 -0000      1.15
+++ lib/modechange.h    1 May 2005 14:25:48 -0000
@@ -27,6 +27,5 @@
 struct mode_change *mode_compile (const char *);
 struct mode_change *mode_create_from_ref (const char *);
 mode_t mode_adjust (mode_t, struct mode_change const *, mode_t);
-void mode_free (struct mode_change *);
 
 #endif
Index: src/install.c
===================================================================
RCS file: /fetish/cu/src/install.c,v
retrieving revision 1.173
diff -p -u -r1.173 install.c
--- src/install.c       28 Apr 2005 16:31:09 -0000      1.173
+++ src/install.c       1 May 2005 14:25:48 -0000
@@ -357,7 +357,7 @@ main (int argc, char **argv)
       if (!change)
        error (EXIT_FAILURE, 0, _("invalid mode %s"), quote (specified_mode));
       mode = mode_adjust (0, change, 0);
-      mode_free (change);
+      free (change);
     }
 
   get_ids ();
Index: src/mkdir.c
===================================================================
RCS file: /fetish/cu/src/mkdir.c,v
retrieving revision 1.93
diff -p -u -r1.93 mkdir.c
--- src/mkdir.c 28 Apr 2005 16:31:09 -0000      1.93
+++ src/mkdir.c 1 May 2005 14:25:48 -0000
@@ -139,7 +139,7 @@ main (int argc, char **argv)
            error (EXIT_FAILURE, 0, _("invalid mode %s"),
                   quote (specified_mode));
          newmode = mode_adjust (S_IRWXUGO, change, umask_value);
-         mode_free (change);
+         free (change);
        }
       else
        umask (umask_value);
Index: src/mkfifo.c
===================================================================
RCS file: /fetish/cu/src/mkfifo.c,v
retrieving revision 1.75
diff -p -u -r1.75 mkfifo.c
--- src/mkfifo.c        28 Apr 2005 16:31:09 -0000      1.75
+++ src/mkfifo.c        1 May 2005 14:25:48 -0000
@@ -119,7 +119,7 @@ main (int argc, char **argv)
       if (!change)
        error (EXIT_FAILURE, 0, _("invalid mode"));
       newmode = mode_adjust (newmode, change, umask (0));
-      mode_free (change);
+      free (change);
     }
 
   for (; optind < argc; ++optind)
Index: src/mknod.c
===================================================================
RCS file: /fetish/cu/src/mknod.c,v
retrieving revision 1.86
diff -p -u -r1.86 mknod.c
--- src/mknod.c 28 Apr 2005 16:31:09 -0000      1.86
+++ src/mknod.c 1 May 2005 14:25:48 -0000
@@ -124,7 +124,7 @@ main (int argc, char **argv)
       if (!change)
        error (EXIT_FAILURE, 0, _("invalid mode"));
       newmode = mode_adjust (newmode, change, umask (0));
-      mode_free (change);
+      free (change);
     }
 
   /* If the number of arguments is 0 or 1,