(make_node_op_equals, mode_compile, mode_create_from_ref, mode_adjust):

Use proper mode_t types and macros.
Don't assume the traditional Unix values for mode bits.
modechange.h now includes sys/types.h.
Include xstrtol.h.
(isodigit, oatoi): Remove.
(S_ISUID, S_ISGID, S_ISVTX, S_IRUSR, S_IWUSR, S_IXUSR, S_IRGRP,
S_IWGRP, S_IXGRP, S_IROTH, S_IWOTH, S_IXOTH, S_IRWXU, S_IRWXG,
S_IRWXO): Define if not defined.
(CHMOD_MODE_BITS): New macro.
(mode_compile): Convert from octal with xstrtoul, not our own routine.

1 files changed, 98 insertions, 57 deletions

diff --git a/lib/modechange.c b/lib/modechange.c
index cbb1bb905b..71432fa427 100644
--- a/lib/modechange.c
+++ b/lib/modechange.c
@@ -28,9 +28,9 @@
 # include <config.h>
 #endif
 
-#include <sys/types.h>
-#include <sys/stat.h>
 #include "modechange.h"
+#include <sys/stat.h>
+#include "xstrtol.h"
 
 #if STDC_HEADERS
 # include <stdlib.h>
@@ -50,34 +50,65 @@ char *malloc ();
 # define S_ISDIR(m) (((m) & S_IFMT) == S_IFDIR)
 #endif
 
-/* Return newly allocated memory to hold one element of type TYPE. */
-#define talloc(type) ((type *) malloc (sizeof (type)))
-
-#define isodigit(c) ((c) >= '0' && (c) <= '7')
+#ifndef S_ISUID
+# define S_ISUID 04000
+#endif
+#ifndef S_ISGID
+# define S_ISGID 04000
+#endif
+#ifndef S_ISVTX
+# define S_ISVTX 01000
+#endif
+#ifndef S_IRUSR
+# define S_IRUSR 0400
+#endif
+#ifndef S_IWUSR
+# define S_IWUSR 0200
+#endif
+#ifndef S_IXUSR
+# define S_IXUSR 0100
+#endif
+#ifndef S_IRGRP
+# define S_IRGRP 0040
+#endif
+#ifndef S_IWGRP
+# define S_IWGRP 0020
+#endif
+#ifndef S_IXGRP
+# define S_IXGRP 0010
+#endif
+#ifndef S_IROTH
+# define S_IROTH 0004
+#endif
+#ifndef S_IWOTH
+# define S_IWOTH 0002
+#endif
+#ifndef S_IXOTH
+# define S_IXOTH 0001
+#endif
+#ifndef S_IRWXU
+# define S_IRWXU 0700
+#endif
+#ifndef S_IRWXG
+# define S_IRWXG 0070
+#endif
+#ifndef S_IRWXO
+# define S_IRWXO 0007
+#endif
 
-/* Return a positive integer containing the value of the ASCII
-   octal number S.  If S is not an octal number, return -1.  */
+/* 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)
 
-static int
-oatoi (const char *s)
-{
-  register int i;
-
-  if (*s == 0)
-    return -1;
-  for (i = 0; isodigit (*s); ++s)
-    i = i * 8 + *s - '0';
-  if (*s)
-    return -1;
-  return i;
-}
+/* 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 (int new_mode)
+make_node_op_equals (mode_t new_mode)
 {
   struct mode_change *p;
   p = talloc (struct mode_change);
@@ -87,7 +118,7 @@ make_node_op_equals (int new_mode)
   p->op = '=';
   p->flags = 0;
   p->value = new_mode;
-  p->affected = 07777;	/* Affect all permissions. */
+  p->affected = CHMOD_MODE_BITS;	/* Affect all permissions. */
   return p;
 }
 
@@ -126,21 +157,21 @@ mode_compile (const char *mode_string, unsigned int masked_ops)
 {
   struct mode_change *head;	/* First element of the linked list. */
   struct mode_change *tail;	/* An element of the linked list. */
-  int i;			/* General purpose temporary. */
-  int umask_value;		/* The umask value (surprise). */
+  unsigned long mode_value;	/* The mode value, if octal.  */
+  char *string_end;		/* Pointer to end of parsed value.  */
+  mode_t umask_value;		/* The umask value (surprise). */
 
   head = NULL;
 #ifdef lint
   tail = NULL;
 #endif
 
-  i = oatoi (mode_string);
-  if (i >= 0)
+  if (xstrtoul (mode_string, &string_end, 8, &mode_value, "") == LONGINT_OK)
     {
       struct mode_change *p;
-      if (i > 07777)
+      if (mode_value > CHMOD_MODE_BITS)
 	return MODE_INVALID;
-      p = make_node_op_equals (i);
+      p = make_node_op_equals ((mode_t) mode_value);
       if (p == NULL)
 	return MODE_MEMORY_EXHAUSTED;
       mode_append_entry (&head, &tail, p);
@@ -155,9 +186,9 @@ mode_compile (const char *mode_string, unsigned int masked_ops)
   do
     {
       /* Which bits in the mode are operated on. */
-      unsigned short affected_bits = 0;
+      mode_t affected_bits = 0;
       /* `affected_bits' modified by umask. */
-      unsigned short affected_masked;
+      mode_t affected_masked;
       /* Operators to actually use umask on. */
       unsigned ops_to_mask = 0;
 
@@ -170,16 +201,16 @@ mode_compile (const char *mode_string, unsigned int masked_ops)
 	switch (*mode_string)
 	  {
 	  case 'u':
-	    affected_bits |= 04700;
+	    affected_bits |= S_ISUID | S_IRWXU;
 	    break;
 	  case 'g':
-	    affected_bits |= 02070;
+	    affected_bits |= S_ISGID | S_IRWXG;
 	    break;
 	  case 'o':
-	    affected_bits |= 01007;
+	    affected_bits |= S_ISVTX | S_IRWXO;
 	    break;
 	  case 'a':
-	    affected_bits |= 07777;
+	    affected_bits |= CHMOD_MODE_BITS;
 	    break;
 	  default:
 	    goto no_more_affected;
@@ -193,7 +224,7 @@ mode_compile (const char *mode_string, unsigned int masked_ops)
       else
 	{
 	  who_specified_p = 0;
-	  affected_bits = 07777;
+	  affected_bits = CHMOD_MODE_BITS;
 	  ops_to_mask = masked_ops;
 	}
 
@@ -238,31 +269,34 @@ mode_compile (const char *mode_string, unsigned int masked_ops)
 	    switch (*mode_string)
 	      {
 	      case 'r':
-		change->value |= 00444 & affected_masked;
+		change->value |= ((S_IRUSR | S_IRGRP | S_IROTH)
+				  & affected_masked);
 		break;
 	      case 'w':
-		change->value |= 00222 & affected_masked;
+		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 |= 00111 & affected_masked;
+		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 |= 06000 & affected_masked;
+		change->value |= (S_ISUID | S_ISGID) & affected_masked;
 		break;
 	      case 't':
 		/* Set the "save text image" bit if `o' is selected. */
-		change->value |= 01000 & affected_masked;
+		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 = 00700;
+		change->value = S_IRWXU;
 		change->flags |= MODE_COPY_EXISTING;
 		break;
 	      case 'g':
@@ -270,7 +304,7 @@ mode_compile (const char *mode_string, unsigned int masked_ops)
 		   on the same file.  */
 		if (change->value)
 		  goto invalid;
-		change->value = 00070;
+		change->value = S_IRWXG;
 		change->flags |= MODE_COPY_EXISTING;
 		break;
 	      case 'o':
@@ -278,7 +312,7 @@ mode_compile (const char *mode_string, unsigned int masked_ops)
 		   on the same file.  */
 		if (change->value)
 		  goto invalid;
-		change->value = 00007;
+		change->value = S_IRWXO;
 		change->flags |= MODE_COPY_EXISTING;
 		break;
 	      default:
@@ -313,7 +347,7 @@ mode_create_from_ref (const char *ref_file)
 
   change->op = '=';
   change->flags = 0;
-  change->affected = 07777;
+  change->affected = CHMOD_MODE_BITS;
   change->value = ref_stats.st_mode;
   change->next = NULL;
 
@@ -325,13 +359,13 @@ mode_create_from_ref (const char *ref_file)
    change affects it even if no execute bits were set in OLDMODE.
    The returned value has the S_IFMT bits cleared. */
 
-unsigned short
-mode_adjust (unsigned int oldmode, const struct mode_change *changes)
+mode_t
+mode_adjust (mode_t oldmode, const struct mode_change *changes)
 {
-  unsigned short newmode;	/* The adjusted mode and one operand. */
-  unsigned short value;		/* The other operand. */
+  mode_t newmode;	/* The adjusted mode and one operand. */
+  mode_t value;		/* The other operand. */
 
-  newmode = oldmode & 07777;
+  newmode = oldmode & CHMOD_MODE_BITS;
 
   for (; changes; changes = changes->next)
     {
@@ -341,15 +375,21 @@ mode_adjust (unsigned int oldmode, const struct mode_change *changes)
 	     the mask `changes->value'. */
 	  value = newmode & changes->value;
 
-	  if (changes->value & 00700)
+	  if (changes->value & S_IRWXU)
 	    /* Copy `u' permissions onto `g' and `o'. */
-	    value |= (value >> 3) | (value >> 6);
-	  else if (changes->value & 00070)
+	    value |= ((value & S_IRUSR ? S_IRGRP | S_IROTH : 0)
+		      | (value & S_IWUSR ? S_IWGRP | S_IROTH : 0)
+		      | (value & S_IXUSR ? S_IXGRP | S_IXOTH : 0));
+	  else if (changes->value & S_IRWXG)
 	    /* Copy `g' permissions onto `u' and `o'. */
-	    value |= (value << 3) | (value >> 3);
+	    value |= ((value & S_IRGRP ? S_IRUSR | S_IROTH : 0)
+		      | (value & S_IWGRP ? S_IWUSR | S_IROTH : 0)
+		      | (value & S_IXGRP ? S_IXUSR | S_IXOTH : 0));
 	  else
 	    /* Copy `o' permissions onto `u' and `g'. */
-	    value |= (value << 3) | (value << 6);
+	    value |= ((value & S_IROTH ? S_IRUSR | S_IRGRP : 0)
+		      | (value & S_IWOTH ? S_IWUSR | S_IRGRP : 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.
@@ -365,8 +405,9 @@ mode_adjust (unsigned int oldmode, const struct mode_change *changes)
 	     directory and no execute bits are already set. */
 	  if ((changes->flags & MODE_X_IF_ANY_X)
 	      && !S_ISDIR (oldmode)
-	      && (newmode & 00111) == 0)
-	    value &= ~00111;	/* Clear the execute bits. */
+	      && (newmode & (S_IXUSR | S_IXGRP | S_IXOTH)) == 0)
+	    /* Clear the execute bits. */
+	    value &= ~ (S_IXUSR | S_IXGRP | S_IXOTH);
 	}
 
       switch (changes->op)