bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

more fixes for symbolic permissions string bugs


From: Paul Eggert
Subject: more fixes for symbolic permissions string bugs
Date: Thu, 28 Apr 2005 09:44:39 -0700
User-agent: Gnus/5.1006 (Gnus v5.10.6) Emacs/21.4 (gnu/linux)

Last week's mkdir -m bugs prompted me to review the code of the
symbolic-permissions parser, and I found a few more bugs.  Unlike the
last week's set I think these bugs are all conservative ones -- i.e.,
they always leave files in a more-protected state than they should be.
(Thank goodness.  :-)

I installed this patch.

2005-04-28  Paul Eggert  <address@hidden>

        * NEWS: Document fixes described below.
        * src/chmod.c (change, umask_value): New static vars.
        (reference_file): Move this static var to inside "main".
        (process_file, process_files): Remove CHANGES arg; now taken from
        static var.  All uses changed.
        (usage): Fix incorrect description of MODE operand.
        (main): For invalid mode usages, output a brief usage message.
        Adjust to new modechange API.
        * install.c (main): Adjust to new modechange API.
        Also, free the mode_change object when done.
        * mkdir.c (main): Likewise.
        * mkfifo.c (main): Likewise.
        * mknod.c (main): Likewise.
        * tests/chmod/equal-X: Check for =xX bug.
        * tests/chmod/equals: Check for =u bug.
        * tests/chmod/usage: Check for u+gr and ug,+x bugs.

Index: NEWS
===================================================================
RCS file: /fetish/cu/NEWS,v
retrieving revision 1.279
diff -p -u -r1.279 NEWS
--- NEWS        26 Apr 2005 16:40:16 -0000      1.279
+++ NEWS        28 Apr 2005 16:39:16 -0000
@@ -86,6 +86,10 @@ GNU coreutils NEWS                      
 
 ** Bug fixes
 
+  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.
+
   dd now computes statistics using a realtime clock (if available)
   rather than the time-of-day clock, to avoid glitches if the
   time-of-day is changed while dd is running.  Also, it avoids
Index: lib/modechange.c
===================================================================
RCS file: /fetish/cu/lib/modechange.c,v
retrieving revision 1.28
diff -p -u -r1.28 modechange.c
--- lib/modechange.c    24 Sep 2004 07:00:59 -0000      1.28
+++ lib/modechange.c    28 Apr 2005 16:27:45 -0000
@@ -1,7 +1,7 @@
 /* modechange.c -- file mode manipulation
 
-   Copyright (C) 1989, 1990, 1997, 1998, 1999, 2001, 2003, 2004 Free
-   Software Foundation, Inc.
+   Copyright (C) 1989, 1990, 1997, 1998, 1999, 2001, 2003, 2004, 2005
+   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
@@ -24,7 +24,7 @@
    We do this instead of re-parsing the ASCII string for each file
    because the compiled form requires less computation to use; when
    changing the mode of many files, this probably results in a
-   performance gain. */
+   performance gain.  */
 
 #if HAVE_CONFIG_H
 # include <config.h>
@@ -32,19 +32,13 @@
 
 #include "modechange.h"
 #include <sys/stat.h>
+#include "stat-macros.h"
+#include "xalloc.h"
 #include "xstrtol.h"
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdlib.h>
 
-#if STAT_MACROS_BROKEN
-# undef S_ISDIR
-#endif
-
-#if !defined(S_ISDIR) && defined(S_IFDIR)
-# define S_ISDIR(m) (((m) & S_IFMT) == S_IFDIR)
-#endif
-
 /* The traditional octal values corresponding to each mode bit.  */
 #define SUID 04000
 #define SGID 02000
@@ -60,127 +54,54 @@
 #define XOTH 00001
 #define ALLM 07777 /* all octal mode bits */
 
-#ifndef S_ISUID
-# define S_ISUID SUID
-#endif
-#ifndef S_ISGID
-# define S_ISGID SGID
-#endif
-#ifndef S_ISVTX
-# define S_ISVTX SVTX
-#endif
-#ifndef S_IRUSR
-# define S_IRUSR RUSR
-#endif
-#ifndef S_IWUSR
-# define S_IWUSR WUSR
-#endif
-#ifndef S_IXUSR
-# define S_IXUSR XUSR
-#endif
-#ifndef S_IRGRP
-# define S_IRGRP RGRP
-#endif
-#ifndef S_IWGRP
-# define S_IWGRP WGRP
-#endif
-#ifndef S_IXGRP
-# define S_IXGRP XGRP
-#endif
-#ifndef S_IROTH
-# define S_IROTH ROTH
-#endif
-#ifndef S_IWOTH
-# define S_IWOTH WOTH
-#endif
-#ifndef S_IXOTH
-# define S_IXOTH XOTH
-#endif
-#ifndef S_IRWXU
-# define S_IRWXU (S_IRUSR | S_IWUSR | S_IXUSR)
-#endif
-#ifndef S_IRWXG
-# define S_IRWXG (S_IRGRP | S_IWGRP | S_IXGRP)
-#endif
-#ifndef S_IRWXO
-# define S_IRWXO (S_IROTH | S_IWOTH | S_IXOTH)
-#endif
-
-/* All the mode bits that can be affected by chmod.  */
-#define CHMOD_MODE_BITS \
-  (S_ISUID | S_ISGID | S_ISVTX | S_IRWXU | S_IRWXG | S_IRWXO)
-
-/* Return newly allocated memory to hold one element of type TYPE. */
-#define talloc(type) ((type *) malloc (sizeof (type)))
-
-/* Create a mode_change entry with the specified `=ddd'-style
-   mode change operation, where NEW_MODE is `ddd'.  Return the
-   new entry, or NULL upon failure.  */
-
-static struct mode_change *
-make_node_op_equals (mode_t new_mode)
-{
-  struct mode_change *p;
-  p = talloc (struct mode_change);
-  if (p == NULL)
-    return p;
-  p->next = NULL;
-  p->op = '=';
-  p->flags = 0;
-  p->value = new_mode;
-  p->affected = CHMOD_MODE_BITS;       /* Affect all permissions. */
-  return p;
-}
-
-/* Append entry E to the end of the link list with the specified
-   HEAD and TAIL.  */
+/* Special operations flags.  */
+enum
+  {
+    /* The typical case.  */
+    MODE_ORDINARY_CHANGE,
+
+    /* In addition to the typical case, affect the execute bits if at
+       least one execute bit is set already, or if the file is a
+       directory.  */
+    MODE_X_IF_ANY_X,
+
+    /* Instead of the typical case, copy some existing permissions for
+       u, g, or o onto the other two.  Which of u, g, or o is copied
+       is determined by which bits are set in the `value' field.  */
+    MODE_COPY_EXISTING
+  };
 
-static void
-mode_append_entry (struct mode_change **head,
-                  struct mode_change **tail,
-                  struct mode_change *e)
+/* Description of a mode change.  */
+struct mode_change
 {
-  if (*head == NULL)
-    *head = *tail = e;
-  else
-    {
-      (*tail)->next = e;
-      *tail = e;
-    }
-}
+  char op;                     /* One of "=+-".  */
+  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
    MODE_STRING, an ASCII string that contains either an octal number
    specifying an absolute mode, or symbolic mode change operations with
    the form:
    [ugoa...][[+-=][rwxXstugo...]...][,...]
-   MASKED_OPS is a bitmask indicating which symbolic mode operators (=+-)
-   should not affect bits set in the umask when no users are given.
-   Operators not selected in MASKED_OPS ignore the umask.
-
-   Return MODE_INVALID if `mode_string' does not contain a valid
-   representation of file mode change operations;
-   return MODE_MEMORY_EXHAUSTED if there is insufficient memory. */
+
+   Return NULL if `mode_string' does not contain a valid
+   representation of file mode change operations.  */
 
 struct mode_change *
-mode_compile (const char *mode_string, unsigned int masked_ops)
+mode_compile (char const *mode_string)
 {
-  struct mode_change *head;    /* First element of the linked list. */
-  struct mode_change *tail;    /* An element of the linked list. */
+  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.  */
-  mode_t umask_value;          /* The umask value (surprise). */
-
-  head = NULL;
-#ifdef lint
-  tail = NULL;
-#endif
 
   if (xstrtoul (mode_string, NULL, 8, &octal_value, "") == LONGINT_OK)
     {
-      struct mode_change *p;
       mode_t mode;
       if (octal_value != (octal_value & ALLM))
-       return MODE_INVALID;
+       return NULL;
 
       /* Help the compiler optimize the usual case where mode_t uses
         the traditional octal representation.  */
@@ -202,166 +123,132 @@ mode_compile (const char *mode_string, u
                          | (octal_value & WOTH ? S_IWOTH : 0)
                          | (octal_value & XOTH ? S_IXOTH : 0)));
 
-      p = make_node_op_equals (mode);
-      if (p == NULL)
-       return MODE_MEMORY_EXHAUSTED;
-      mode_append_entry (&head, &tail, p);
+      head = xmalloc (sizeof *head);
+      head->op = '=';
+      head->flag = MODE_ORDINARY_CHANGE;
+      head->affected = CHMOD_MODE_BITS;
+      head->value = mode;
+      head->next = NULL;
       return head;
     }
 
-  umask_value = umask (0);
-  umask (umask_value);         /* Restore the old value. */
-
-  /* One loop iteration for each "ugoa...=+-rwxXstugo...[=+-rwxXstugo...]". */
+  /* One loop iteration for each `[ugoa]*([-+=]([rwxXst]*|[ugo]))+'.  */
   for (;; mode_string++)
     {
-      /* Which bits in the mode are operated on. */
-      mode_t affected_bits = 0;
-      /* `affected_bits' modified by umask. */
-      mode_t affected_masked;
-      /* Operators to actually use umask on. */
-      unsigned int ops_to_mask = 0;
+      /* Which bits in the mode are operated on.  */
+      mode_t affected = 0;
 
-      bool who_specified_p;
-
-      /* Turn on all the bits in `affected_bits' for each group given. */
+      /* Turn on all the bits in `affected' for each group given.  */
       for (;; mode_string++)
        switch (*mode_string)
          {
          case 'u':
-           affected_bits |= S_ISUID | S_IRWXU;
+           affected |= S_ISUID | S_IRWXU;
            break;
          case 'g':
-           affected_bits |= S_ISGID | S_IRWXG;
+           affected |= S_ISGID | S_IRWXG;
            break;
          case 'o':
-           affected_bits |= S_ISVTX | S_IRWXO;
+           affected |= S_ISVTX | S_IRWXO;
            break;
          case 'a':
-           affected_bits |= CHMOD_MODE_BITS;
+           affected |= CHMOD_MODE_BITS;
            break;
-         default:
+         case '=': case '+': case '-':
            goto no_more_affected;
+         default:
+           goto invalid;
          }
+    no_more_affected:;
 
-    no_more_affected:
-      /* If none specified, affect all bits, except perhaps those
-        set in the umask. */
-      if (affected_bits)
-       who_specified_p = true;
-      else
+      do
        {
-         who_specified_p = false;
-         affected_bits = CHMOD_MODE_BITS;
-         ops_to_mask = masked_ops;
-       }
+         char op = *mode_string++;
+         mode_t value;
+         char flag = MODE_COPY_EXISTING;
+         struct mode_change *change;
 
-      while (*mode_string == '=' || *mode_string == '+' || *mode_string == '-')
-       {
-         struct mode_change *change = talloc (struct mode_change);
-         if (change == NULL)
+         switch (*mode_string++)
            {
-             mode_free (head);
-             return MODE_MEMORY_EXHAUSTED;
+           case 'u':
+             /* Set the affected bits to the value of the `u' bits
+                on the same file.  */
+             value = S_IRWXU;
+             break;
+           case 'g':
+             /* Set the affected bits to the value of the `g' bits
+                on the same file.  */
+             value = S_IRWXG;
+             break;
+           case 'o':
+             /* Set the affected bits to the value of the `o' bits
+                on the same file.  */
+             value = S_IRWXO;
+             break;
+
+           default:
+             value = 0;
+             flag = MODE_ORDINARY_CHANGE;
+
+             for (mode_string--;; mode_string++)
+               switch (*mode_string)
+                 {
+                 case 'r':
+                   value |= S_IRUSR | S_IRGRP | S_IROTH;
+                   break;
+                 case 'w':
+                   value |= S_IWUSR | S_IWGRP | S_IWOTH;
+                   break;
+                 case 'x':
+                   value |= S_IXUSR | S_IXGRP | S_IXOTH;
+                   break;
+                 case 'X':
+                   flag = MODE_X_IF_ANY_X;
+                   break;
+                 case 's':
+                   /* Set the setuid/gid bits if `u' or `g' is selected.  */
+                   value |= S_ISUID | S_ISGID;
+                   break;
+                 case 't':
+                   /* Set the "save text image" bit if `o' is selected.  */
+                   value |= S_ISVTX;
+                   break;
+                 default:
+                   goto no_more_values;
+                 }
+           no_more_values:;
            }
 
-         change->next = NULL;
-         change->op = *mode_string;    /* One of "=+-". */
-         affected_masked = affected_bits;
-
-         /* Per the Single Unix Spec, if `who' is not specified and the
-            `=' operator is used, then clear all the bits first.  */
-         if (!who_specified_p &&
-             ops_to_mask & (*mode_string == '=' ? MODE_MASK_EQUALS : 0))
-           {
-             struct mode_change *p = make_node_op_equals (0);
-             if (p == NULL)
-               return MODE_MEMORY_EXHAUSTED;
-             mode_append_entry (&head, &tail, p);
-           }
+         change = xmalloc (sizeof *change);
+         change->op = op;
+         change->flag = flag;
+         change->affected = affected;
+         change->value = value;
 
-         if (ops_to_mask & (*mode_string == '=' ? MODE_MASK_EQUALS
-                            : *mode_string == '+' ? MODE_MASK_PLUS
-                            : MODE_MASK_MINUS))
-           affected_masked &= ~umask_value;
-         change->affected = affected_masked;
-         change->value = 0;
-         change->flags = 0;
-
-         /* Add the element to the tail of the list, so the operations
-            are performed in the correct order. */
-         mode_append_entry (&head, &tail, change);
-
-         /* Set `value' according to the bits set in `affected_masked'. */
-         for (++mode_string;; ++mode_string)
-           switch (*mode_string)
-             {
-             case 'r':
-               change->value |= ((S_IRUSR | S_IRGRP | S_IROTH)
-                                 & affected_masked);
-               break;
-             case 'w':
-               change->value |= ((S_IWUSR | S_IWGRP | S_IWOTH)
-                                 & affected_masked);
-               break;
-             case 'X':
-               change->flags |= MODE_X_IF_ANY_X;
-               /* Fall through. */
-             case 'x':
-               change->value |= ((S_IXUSR | S_IXGRP | S_IXOTH)
-                                 & affected_masked);
-               break;
-             case 's':
-               /* Set the setuid/gid bits if `u' or `g' is selected. */
-               change->value |= (S_ISUID | S_ISGID) & affected_masked;
-               break;
-             case 't':
-               /* Set the "save text image" bit if `o' is selected. */
-               change->value |= S_ISVTX & affected_masked;
-               break;
-             case 'u':
-               /* Set the affected bits to the value of the `u' bits
-                  on the same file.  */
-               if (change->value)
-                 goto invalid;
-               change->value = S_IRWXU;
-               change->flags |= MODE_COPY_EXISTING;
-               break;
-             case 'g':
-               /* Set the affected bits to the value of the `g' bits
-                  on the same file.  */
-               if (change->value)
-                 goto invalid;
-               change->value = S_IRWXG;
-               change->flags |= MODE_COPY_EXISTING;
-               break;
-             case 'o':
-               /* Set the affected bits to the value of the `o' bits
-                  on the same file.  */
-               if (change->value)
-                 goto invalid;
-               change->value = S_IRWXO;
-               change->flags |= MODE_COPY_EXISTING;
-               break;
-             default:
-               goto no_more_values;
-             }
-       no_more_values:;
+         *tail = change;
+         tail = &change->next;
        }
+      while (*mode_string == '=' || *mode_string == '+'
+            || *mode_string == '-');
 
       if (*mode_string != ',')
        break;
     }
 
   if (*mode_string == 0)
-    return head;
+    {
+      *tail = NULL;
+      return head;
+    }
+
 invalid:
+  *tail = NULL;
   mode_free (head);
-  return MODE_INVALID;
+  return NULL;
 }
 
 /* Return a file mode change operation that sets permissions to match those
-   of REF_FILE.  Return MODE_BAD_REFERENCE if REF_FILE can't be accessed.  */
+   of REF_FILE.  Return NULL (setting errno) if REF_FILE can't be accessed.  */
 
 struct mode_change *
 mode_create_from_ref (const char *ref_file)
@@ -369,16 +256,12 @@ mode_create_from_ref (const char *ref_fi
   struct mode_change *change;  /* the only change element */
   struct stat ref_stats;
 
-  if (stat (ref_file, &ref_stats))
-    return MODE_BAD_REFERENCE;
-
-  change = talloc (struct mode_change);
-
-  if (change == NULL)
-    return MODE_MEMORY_EXHAUSTED;
+  if (stat (ref_file, &ref_stats) != 0)
+    return NULL;
 
+  change = xmalloc (sizeof *change);
   change->op = '=';
-  change->flags = 0;
+  change->flag = MODE_ORDINARY_CHANGE;
   change->affected = CHMOD_MODE_BITS;
   change->value = ref_stats.st_mode;
   change->next = NULL;
@@ -387,90 +270,83 @@ mode_create_from_ref (const char *ref_fi
 }
 
 /* Return file mode OLDMODE, adjusted as indicated by the list of change
-   operations CHANGES.  If OLDMODE is a directory, the type `X'
+   operations CHANGES, which are interpreted assuming the umask is
+   UMASK_VALUE.  If OLDMODE is a directory, the type `X'
    change affects it even if no execute bits were set in OLDMODE.
-   The returned value has the S_IFMT bits cleared. */
+   The returned value has the S_IFMT bits cleared.  */
 
 mode_t
-mode_adjust (mode_t oldmode, const struct mode_change *changes)
+mode_adjust (mode_t oldmode, struct mode_change const *changes,
+            mode_t umask_value)
 {
-  mode_t newmode;      /* The adjusted mode and one operand. */
-  mode_t value;                /* The other operand. */
-
-  newmode = oldmode & CHMOD_MODE_BITS;
+  /* The adjusted mode.  */
+  mode_t newmode = oldmode & CHMOD_MODE_BITS;
 
   for (; changes; changes = changes->next)
     {
-      if (changes->flags & MODE_COPY_EXISTING)
-       {
-         /* Isolate in `value' the bits in `newmode' to copy, given in
-            the mask `changes->value'. */
-         value = newmode & changes->value;
-
-         if (changes->value & S_IRWXU)
-           /* Copy `u' permissions onto `g' and `o'. */
-           value |= (  (value & S_IRUSR ? S_IRGRP | S_IROTH : 0)
-                     | (value & S_IWUSR ? S_IWGRP | S_IWOTH : 0)
-                     | (value & S_IXUSR ? S_IXGRP | S_IXOTH : 0));
-         else if (changes->value & S_IRWXG)
-           /* Copy `g' permissions onto `u' and `o'. */
-           value |= (  (value & S_IRGRP ? S_IRUSR | S_IROTH : 0)
-                     | (value & S_IWGRP ? S_IWUSR | S_IWOTH : 0)
-                     | (value & S_IXGRP ? S_IXUSR | S_IXOTH : 0));
-         else
-           /* Copy `o' permissions onto `u' and `g'. */
-           value |= (  (value & S_IROTH ? S_IRUSR | S_IRGRP : 0)
-                     | (value & S_IWOTH ? S_IWUSR | S_IWGRP : 0)
-                     | (value & S_IXOTH ? S_IXUSR | S_IXGRP : 0));
-
-         /* In order to change only `u', `g', or `o' permissions,
-            or some combination thereof, clear unselected bits.
-            This cannot be done in mode_compile because the value
-            to which the `changes->affected' mask is applied depends
-            on the old mode of each file. */
-         value &= changes->affected;
-       }
-      else
+      mode_t affected = changes->affected;
+      mode_t value = changes->value;
+
+      switch (changes->flag)
        {
-         value = changes->value;
-         /* If `X', do not affect the execute bits if the file is not a
-            directory and no execute bits are already set. */
-         if ((changes->flags & MODE_X_IF_ANY_X)
-             && !S_ISDIR (oldmode)
-             && (newmode & (S_IXUSR | S_IXGRP | S_IXOTH)) == 0)
-           /* Clear the execute bits. */
-           value &= ~ (S_IXUSR | S_IXGRP | S_IXOTH);
+       case MODE_ORDINARY_CHANGE:
+         break;
+
+       case MODE_COPY_EXISTING:
+         /* Isolate in `value' the bits in `newmode' to copy.  */
+         value &= newmode;
+
+         /* Copy the isolated bits to the other two parts.  */
+         value |= ((value & (S_IRUSR | S_IRGRP | S_IROTH)
+                    ? S_IRUSR | S_IRGRP | S_IROTH : 0)
+                   | (value & (S_IWUSR | S_IWGRP | S_IWOTH)
+                      ? S_IWUSR | S_IWGRP | S_IWOTH : 0)
+                   | (value & (S_IXUSR | S_IXGRP | S_IXOTH)
+                      ? S_IXUSR | S_IXGRP | S_IXOTH : 0));
+         break;
+
+       case MODE_X_IF_ANY_X:
+         /* Affect the execute bits if execute bits are already set
+            or if the file is a directory.  */
+         if ((newmode & (S_IXUSR | S_IXGRP | S_IXOTH)) || S_ISDIR (oldmode))
+           value |= S_IXUSR | S_IXGRP | S_IXOTH;
+         break;
        }
 
+      /* If WHO was specified, limit the change to the affected bits.
+        Otherwise, apply the umask.  */
+      value &= (affected ? affected : ~umask_value);
+
       switch (changes->op)
        {
        case '=':
-         /* Preserve the previous values in `newmode' of bits that are
-            not affected by this change operation. */
-         newmode = (newmode & ~changes->affected) | value;
-         break;
+         /* If WHO was specified, preserve the previous values of
+            bits that are not affected by this change operation.
+            Otherwise, clear all the bits.  */
+         newmode = (affected ? newmode & ~affected : 0);
+         /* Fall through.  */
        case '+':
          newmode |= value;
          break;
+
        case '-':
          newmode &= ~value;
          break;
        }
     }
+
   return newmode;
 }
 
 /* Free the memory used by the list of file mode change operations
-   CHANGES. */
+   CHANGES.  */
 
 void
-mode_free (register struct mode_change *changes)
+mode_free (struct mode_change *changes)
 {
-  register struct mode_change *next;
-
   while (changes)
     {
-      next = changes->next;
+      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.14
diff -p -u -r1.14 modechange.h
--- lib/modechange.h    2 Aug 2004 22:58:37 -0000       1.14
+++ lib/modechange.h    28 Apr 2005 16:27:45 -0000
@@ -1,5 +1,7 @@
 /* modechange.h -- definitions for file mode manipulation
-   Copyright (C) 1989, 1990, 1997, 2003, 2004 Free Software Foundation, Inc.
+
+   Copyright (C) 1989, 1990, 1997, 2003, 2004, 2005 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
@@ -22,38 +24,9 @@
 
 # include <sys/types.h>
 
-/* Affect the execute bits only if at least one execute bit is set already,
-   or if the file is a directory. */
-# define MODE_X_IF_ANY_X 01
-
-/* If set, copy some existing permissions for u, g, or o onto the other two.
-   Which of u, g, or o is copied is determined by which bits are set in the
-   `value' field. */
-# define MODE_COPY_EXISTING 02
-
-struct mode_change
-{
-  char op;                     /* One of "=+-". */
-  char flags;                  /* Special operations. */
-  mode_t affected;             /* Set for u/g/o/s/s/t, if to be affected. */
-  mode_t value;                        /* Bits to add/remove. */
-  struct mode_change *next;    /* Link to next change in list. */
-};
-
-/* Masks for mode_compile argument. */
-# define MODE_MASK_EQUALS 1
-# define MODE_MASK_PLUS 2
-# define MODE_MASK_MINUS 4
-# define MODE_MASK_ALL (MODE_MASK_EQUALS | MODE_MASK_PLUS | MODE_MASK_MINUS)
-
-/* Error return values for mode_compile. */
-# define MODE_INVALID (struct mode_change *) 0
-# define MODE_MEMORY_EXHAUSTED (struct mode_change *) 1
-# define MODE_BAD_REFERENCE (struct mode_change *) 2
-
-struct mode_change *mode_compile (const char *, unsigned int);
+struct mode_change *mode_compile (const char *);
 struct mode_change *mode_create_from_ref (const char *);
-mode_t mode_adjust (mode_t, const struct mode_change *);
+mode_t mode_adjust (mode_t, struct mode_change const *, mode_t);
 void mode_free (struct mode_change *);
 
 #endif
Index: src/chmod.c
===================================================================
RCS file: /fetish/cu/src/chmod.c,v
retrieving revision 1.108
diff -p -u -r1.108 chmod.c
--- src/chmod.c 28 Mar 2005 17:46:55 -0000      1.108
+++ src/chmod.c 28 Apr 2005 16:27:45 -0000
@@ -60,6 +60,12 @@ enum Verbosity
 /* The name the program was run with. */
 char *program_name;
 
+/* The desired change to the mode.  */
+static struct mode_change *change;
+
+/* The initial umask value, if it might be needed.  */
+static mode_t umask_value;
+
 /* If true, change the modes of directories recursively. */
 static bool recurse;
 
@@ -69,10 +75,6 @@ static bool force_silent;
 /* Level of verbosity.  */
 static enum Verbosity verbosity = V_off;
 
-/* The argument to the --reference option.  Use the owner and group IDs
-   of this file.  This file must exist.  */
-static char *reference_file;
-
 /* Pointer to the device and inode numbers of `/', when --recursive.
    Otherwise NULL.  */
 static struct dev_ino *root_dev_ino;
@@ -164,12 +166,12 @@ describe_change (const char *file, mode_
          (unsigned long int) (mode & CHMOD_MODE_BITS), &perms[1]);
 }
 
-/* Change the mode of FILE according to the list of operations CHANGES.
+/* Change the mode of FILE.
    Return true if successful.  This function is called
    once for every file system object that fts encounters.  */
 
 static bool
-process_file (FTS *fts, FTSENT *ent, const struct mode_change *changes)
+process_file (FTS *fts, FTSENT *ent)
 {
   char const *file_full_name = ent->fts_path;
   char const *file = ent->fts_accpath;
@@ -215,7 +217,7 @@ process_file (FTS *fts, FTSENT *ent, con
 
   if (do_chmod)
     {
-      new_mode = mode_adjust (file_stats->st_mode, changes);
+      new_mode = mode_adjust (file_stats->st_mode, change, umask_value);
 
       if (S_ISLNK (file_stats->st_mode))
        symlink_changed = false;
@@ -253,12 +255,11 @@ process_file (FTS *fts, FTSENT *ent, con
 }
 
 /* Recursively change the modes of the specified FILES (the last entry
-   of which is NULL) according to the list of operations CHANGES.
-   BIT_FLAGS controls how fts works.
+   of which is NULL).  BIT_FLAGS controls how fts works.
    Return true if successful.  */
 
 static bool
-process_files (char **files, int bit_flags, const struct mode_change *changes)
+process_files (char **files, int bit_flags)
 {
   bool ok = true;
 
@@ -280,7 +281,7 @@ process_files (char **files, int bit_fla
          break;
        }
 
-      ok &= process_file (fts, ent, changes);
+      ok &= process_file (fts, ent);
     }
 
   /* Ignore failure, since the only way it can do so is in failing to
@@ -324,8 +325,7 @@ Change the mode of each FILE to MODE.\n\
       fputs (VERSION_OPTION_DESCRIPTION, stdout);
       fputs (_("\
 \n\
-Each MODE is one or more of the letters ugoa, one of the symbols +-= and\n\
-one or more of the letters rwxXstugo.\n\
+Each MODE is of the form `[ugoa]*([-+=]([rwxXst]*|[ugo]))+'.\n\
 "), stdout);
       printf (_("\nReport bugs to <%s>.\n"), PACKAGE_BUGREPORT);
     }
@@ -338,12 +338,12 @@ one or more of the letters rwxXstugo.\n\
 int
 main (int argc, char **argv)
 {
-  struct mode_change *changes;
   char *mode = NULL;
   size_t mode_len = 0;
   size_t mode_alloc = 0;
   bool ok;
   bool preserve_root = false;
+  char const *reference_file = NULL;
   int c;
 
   initialize_main (&argc, &argv);
@@ -428,8 +428,10 @@ main (int argc, char **argv)
   if (reference_file)
     {
       if (mode)
-       error (EXIT_FAILURE, 0,
-              _("cannot combine mode and --reference options"));
+       {
+         error (0, 0, _("cannot combine mode and --reference options"));
+         usage (EXIT_FAILURE);
+       }
     }
   else
     {
@@ -446,16 +448,23 @@ main (int argc, char **argv)
       usage (EXIT_FAILURE);
     }
 
-  changes = (reference_file ? mode_create_from_ref (reference_file)
-            : mode_compile (mode, MODE_MASK_ALL));
-
-  if (changes == MODE_INVALID)
-    error (EXIT_FAILURE, 0, _("invalid mode: %s"), quote (mode));
-  else if (changes == MODE_MEMORY_EXHAUSTED)
-    xalloc_die ();
-  else if (changes == MODE_BAD_REFERENCE)
-    error (EXIT_FAILURE, errno, _("failed to get attributes of %s"),
-          quote (reference_file));
+  if (reference_file)
+    {
+      change = mode_create_from_ref (reference_file);
+      if (!change)
+       error (EXIT_FAILURE, errno, _("failed to get attributes of %s"),
+              quote (reference_file));
+    }
+  else
+    {
+      change = mode_compile (mode);
+      if (!change)
+       {
+         error (0, 0, _("invalid mode: %s"), quote (mode));
+         usage (EXIT_FAILURE);
+       }
+      umask_value = umask (0);
+    }
 
   if (recurse & preserve_root)
     {
@@ -470,7 +479,7 @@ main (int argc, char **argv)
       root_dev_ino = NULL;
     }
 
-  ok = process_files (argv + optind, FTS_COMFOLLOW, changes);
+  ok = process_files (argv + optind, FTS_COMFOLLOW);
 
   exit (ok ? EXIT_SUCCESS : EXIT_FAILURE);
 }
Index: src/install.c
===================================================================
RCS file: /fetish/cu/src/install.c,v
retrieving revision 1.172
diff -p -u -r1.172 install.c
--- src/install.c       26 Nov 2004 07:39:32 -0000      1.172
+++ src/install.c       28 Apr 2005 16:27:45 -0000
@@ -353,12 +353,11 @@ main (int argc, char **argv)
 
   if (specified_mode)
     {
-      struct mode_change *change = mode_compile (specified_mode, 0);
-      if (change == MODE_INVALID)
+      struct mode_change *change = mode_compile (specified_mode);
+      if (!change)
        error (EXIT_FAILURE, 0, _("invalid mode %s"), quote (specified_mode));
-      else if (change == MODE_MEMORY_EXHAUSTED)
-       xalloc_die ();
-      mode = mode_adjust (0, change);
+      mode = mode_adjust (0, change, 0);
+      mode_free (change);
     }
 
   get_ids ();
Index: src/mkdir.c
===================================================================
RCS file: /fetish/cu/src/mkdir.c,v
retrieving revision 1.92
diff -p -u -r1.92 mkdir.c
--- src/mkdir.c 22 Apr 2005 23:52:35 -0000      1.92
+++ src/mkdir.c 28 Apr 2005 16:27:46 -0000
@@ -125,22 +125,24 @@ main (int argc, char **argv)
     }
 
   newmode = S_IRWXUGO;
-  if (specified_mode)
-    {
-      struct mode_change *change = mode_compile (specified_mode, 
MODE_MASK_ALL);
-      if (change == MODE_INVALID)
-       error (EXIT_FAILURE, 0, _("invalid mode %s"), quote (specified_mode));
-      else if (change == MODE_MEMORY_EXHAUSTED)
-       xalloc_die ();
-      newmode = mode_adjust (newmode, change);
-    }
 
   if (specified_mode || create_parents)
     {
       mode_t umask_value = umask (0);
-      if (! specified_mode)
-       umask (umask_value);
+
       parent_mode = (S_IRWXUGO & ~umask_value) | (S_IWUSR | S_IXUSR);
+
+      if (specified_mode)
+       {
+         struct mode_change *change = mode_compile (specified_mode);
+         if (!change)
+           error (EXIT_FAILURE, 0, _("invalid mode %s"),
+                  quote (specified_mode));
+         newmode = mode_adjust (S_IRWXUGO, change, umask_value);
+         mode_free (change);
+       }
+      else
+       umask (umask_value);
     }
 
   for (; optind < argc; ++optind)
Index: src/mkfifo.c
===================================================================
RCS file: /fetish/cu/src/mkfifo.c,v
retrieving revision 1.74
diff -p -u -r1.74 mkfifo.c
--- src/mkfifo.c        22 Apr 2005 23:53:13 -0000      1.74
+++ src/mkfifo.c        28 Apr 2005 16:27:46 -0000
@@ -75,7 +75,6 @@ int
 main (int argc, char **argv)
 {
   mode_t newmode;
-  struct mode_change *change;
   const char *specified_mode;
   int exit_status = EXIT_SUCCESS;
   int optc;
@@ -116,13 +115,11 @@ main (int argc, char **argv)
   newmode = (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
   if (specified_mode)
     {
-      change = mode_compile (specified_mode, MODE_MASK_ALL);
-      if (change == MODE_INVALID)
+      struct mode_change *change = mode_compile (specified_mode);
+      if (!change)
        error (EXIT_FAILURE, 0, _("invalid mode"));
-      else if (change == MODE_MEMORY_EXHAUSTED)
-       xalloc_die ();
-      newmode = mode_adjust (newmode, change);
-      umask (0);
+      newmode = mode_adjust (newmode, change, umask (0));
+      mode_free (change);
     }
 
   for (; optind < argc; ++optind)
Index: src/mknod.c
===================================================================
RCS file: /fetish/cu/src/mknod.c,v
retrieving revision 1.85
diff -p -u -r1.85 mknod.c
--- src/mknod.c 22 Apr 2005 23:53:33 -0000      1.85
+++ src/mknod.c 28 Apr 2005 16:27:46 -0000
@@ -88,7 +88,6 @@ int
 main (int argc, char **argv)
 {
   mode_t newmode;
-  struct mode_change *change;
   const char *specified_mode;
   int optc;
   int expected_operands;
@@ -121,13 +120,11 @@ main (int argc, char **argv)
   newmode = (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
   if (specified_mode)
     {
-      change = mode_compile (specified_mode, MODE_MASK_ALL);
-      if (change == MODE_INVALID)
+      struct mode_change *change = mode_compile (specified_mode);
+      if (!change)
        error (EXIT_FAILURE, 0, _("invalid mode"));
-      else if (change == MODE_MEMORY_EXHAUSTED)
-       xalloc_die ();
-      newmode = mode_adjust (newmode, change);
-      umask (0);
+      newmode = mode_adjust (newmode, change, umask (0));
+      mode_free (change);
     }
 
   /* If the number of arguments is 0 or 1,
Index: tests/chmod/equal-x
===================================================================
RCS file: /fetish/cu/tests/chmod/equal-x,v
retrieving revision 1.7
diff -p -u -r1.7 equal-x
--- tests/chmod/equal-x 23 Jun 2004 15:07:04 -0000      1.7
+++ tests/chmod/equal-x 28 Apr 2005 16:27:46 -0000
@@ -16,18 +16,20 @@ cd $tmp || framework_failure=1
 
 file=f
 touch $file || framework_failure=1
-chmod 444 $file || framework_failure=1
 
 if test $framework_failure = 1; then
   echo 'failure in testing framework'
   (exit 1); exit 1
 fi
 
+fail=0
 umask 005
-chmod =x $file
-case "`ls -l $file`" in
-  ---x--x---*) fail=0 ;;
-  *) fail=1; ls -l $file ;;
-esac
+for mode in =x =xX =Xx =x,=X =X,=x; do
+  chmod a=r,$mode $file || fail=1
+  case "`ls -l $file`" in
+    ---x--x---*) ;;
+    *) fail=1; echo "after \`chmod $mode $file':"; ls -l $file ;;
+  esac
+done
 
 (exit $fail); exit $fail
Index: tests/chmod/equals
===================================================================
RCS file: /fetish/cu/tests/chmod/equals,v
retrieving revision 1.3
diff -p -u -r1.3 equals
--- tests/chmod/equals  23 Jun 2004 15:07:04 -0000      1.3
+++ tests/chmod/equals  28 Apr 2005 16:27:46 -0000
@@ -1,6 +1,7 @@
 #!/bin/sh
 # Make sure chmod mode arguments of the form A=B work properly.
 # Before fileutils-4.1.2, some of them didn't.
+# Also, before coreutils-5.3.1, =[ugo] sometimes didn't work.
 
 if test "$VERBOSE" = yes; then
   set -x
@@ -38,4 +39,9 @@ for src in u g o; do
   done
 done
 
+umask 027
+chmod a=,u=rwx,=u f || fail=1
+set _ `ls -l f`; shift; actual_perms=$1
+test "$actual_perms" = "-rwxr-x---" || fail=1
+
 (exit $fail); exit $fail
Index: tests/chmod/usage
===================================================================
RCS file: /fetish/cu/tests/chmod/usage,v
retrieving revision 1.1
diff -p -u -r1.1 usage
--- tests/chmod/usage   24 Sep 2004 23:34:07 -0000      1.1
+++ tests/chmod/usage   28 Apr 2005 16:27:46 -0000
@@ -54,6 +54,8 @@ cases='
   f --       :
   f -w       : f
   f f        :
+  u+gr f     :
+  ug,+x f    :
 '
 
 all_files=`echo "$cases" | sed 's/.*://'`




reply via email to

[Prev in Thread] Current Thread [Next in Thread]