- Fixed a logic problem for an ACLS_NEED_MASK system where group_obj
authorWayne Davison <wayned@samba.org>
Mon, 20 Mar 2006 00:59:12 +0000 (00:59 +0000)
committerWayne Davison <wayned@samba.org>
Mon, 20 Mar 2006 00:59:12 +0000 (00:59 +0000)
  would not get reconstructed correctly for simple ACL sets.
- Fixed a build problem for an ACLS_NEED_MASK system.
- Avoid sending redundant group/mask info when possible.
- Renamed NO_ENTRY define ACL_NO_ENTRY.

acls.diff

index f6b2f31..474056a 100644 (file)
--- a/acls.diff
+++ b/acls.diff
@@ -32,7 +32,7 @@ This code does not yet itemize changes in ACL information (see --itemize).
  popt_OBJS=popt/findme.o  popt/popt.o  popt/poptconfig.o \
 --- old/acls.c
 +++ new/acls.c
-@@ -0,0 +1,1264 @@
+@@ -0,0 +1,1297 @@
 +/* -*- c-file-style: "linux" -*-
 +   Copyright (C) Andrew Tridgell 1996
 +   Copyright (C) Paul Mackerras 1996
@@ -77,50 +77,62 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +      id_access *idas;
 +} ida_list;
 +
-+#define NO_ENTRY ((uchar)0x80)
++#define ACL_NO_ENTRY ((uchar)0x80)
 +typedef struct {
 +      ida_list users;
 +      ida_list groups;
-+      /* These will be NO_ENTRY if there's no such entry. */
++      /* These will be ACL_NO_ENTRY if there's no such entry. */
 +      uchar user_obj;
 +      uchar group_obj;
 +      uchar mask;
 +      uchar other;
 +} rsync_acl;
 +
-+static const rsync_acl rsync_acl_initializer =
-+      { {0, 0, NULL}, {0, 0, NULL}, NO_ENTRY, NO_ENTRY, NO_ENTRY, NO_ENTRY};
++static const rsync_acl rsync_acl_initializer = {
++      {0, 0, NULL}, {0, 0, NULL},
++      ACL_NO_ENTRY, ACL_NO_ENTRY, ACL_NO_ENTRY, ACL_NO_ENTRY
++};
 +
 +#define OTHER_TYPE(t) (SMB_ACL_TYPE_ACCESS+SMB_ACL_TYPE_DEFAULT-(t))
 +#define BUMP_TYPE(t) ((t = OTHER_TYPE(t)) == SMB_ACL_TYPE_DEFAULT)
 +
 +/* a few useful calculations */
 +
-+static int rsync_acl_count_entries(const rsync_acl *racl)
++static int count_racl_entries(const rsync_acl *racl)
++{
++      return racl->users.count + racl->groups.count
++           + (racl->user_obj != ACL_NO_ENTRY)
++           + (racl->group_obj != ACL_NO_ENTRY)
++           + (racl->mask != ACL_NO_ENTRY)
++           + (racl->other != ACL_NO_ENTRY);
++}
++
++static int calc_sacl_entries(const rsync_acl *racl)
 +{
 +      return racl->users.count + racl->groups.count
-+           + (racl->user_obj != NO_ENTRY)
-+           + (racl->group_obj != NO_ENTRY)
-+           + (racl->mask != NO_ENTRY)
-+           + (racl->other != NO_ENTRY);
++#ifdef ACLS_NEED_MASK
++           + 4;
++#else
++           + (racl->mask != ACL_NO_ENTRY) + 3;
++#endif
 +}
 +
 +static int rsync_acl_get_perms(const rsync_acl *racl)
 +{
-+      /* Note that (NO_ENTRY & 7) is 0. */
++      /* Note that (ACL_NO_ENTRY & 7) is 0. */
 +      return ((racl->user_obj & 7) << 6)
-+           + (((racl->mask != NO_ENTRY ? racl->mask : racl->group_obj) & 7) << 3)
++           + (((racl->mask != ACL_NO_ENTRY ? racl->mask : racl->group_obj) & 7) << 3)
 +           + (racl->other & 7);
 +}
 +
 +static void rsync_acl_strip_perms(rsync_acl *racl)
 +{
-+      racl->user_obj = NO_ENTRY;
-+      if (racl->mask == NO_ENTRY)
-+              racl->group_obj = NO_ENTRY;
++      racl->user_obj = ACL_NO_ENTRY;
++      if (racl->mask == ACL_NO_ENTRY)
++              racl->group_obj = ACL_NO_ENTRY;
 +      else
-+              racl->mask = NO_ENTRY;
-+      racl->other = NO_ENTRY;
++              racl->mask = ACL_NO_ENTRY;
++      racl->other = ACL_NO_ENTRY;
 +}
 +
 +static void expand_ida_list(ida_list *idal)
@@ -205,7 +217,7 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +              /* continue == done with entry; break == store in given idal */
 +              switch (tag_type) {
 +              case SMB_ACL_USER_OBJ:
-+                      if (racl->user_obj == NO_ENTRY)
++                      if (racl->user_obj == ACL_NO_ENTRY)
 +                              racl->user_obj = access;
 +                      else
 +                              rprintf(FINFO, "unpack_smb_acl: warning: duplicate USER_OBJ entry ignored\n");
@@ -214,7 +226,7 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +                      idal = &racl->users;
 +                      break;
 +              case SMB_ACL_GROUP_OBJ:
-+                      if (racl->group_obj == NO_ENTRY)
++                      if (racl->group_obj == ACL_NO_ENTRY)
 +                              racl->group_obj = access;
 +                      else
 +                              rprintf(FINFO, "unpack_smb_acl: warning: duplicate GROUP_OBJ entry ignored\n");
@@ -223,13 +235,13 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +                      idal = &racl->groups;
 +                      break;
 +              case SMB_ACL_MASK:
-+                      if (racl->mask == NO_ENTRY)
++                      if (racl->mask == ACL_NO_ENTRY)
 +                              racl->mask = access;
 +                      else
 +                              rprintf(FINFO, "unpack_smb_acl: warning: duplicate MASK entry ignored\n");
 +                      continue;
 +              case SMB_ACL_OTHER:
-+                      if (racl->other == NO_ENTRY)
++                      if (racl->other == ACL_NO_ENTRY)
 +                              racl->other = access;
 +                      else
 +                              rprintf(FINFO, "unpack_smb_acl: warning: duplicate OTHER entry ignored\n");
@@ -290,9 +302,9 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +static BOOL rsync_acl_extended_parts_equal(const rsync_acl *racl1, const rsync_acl *racl2)
 +{
 +      /* We ignore any differences that chmod() can take care of. */
-+      if ((racl1->mask ^ racl2->mask) & NO_ENTRY)
++      if ((racl1->mask ^ racl2->mask) & ACL_NO_ENTRY)
 +              return False;
-+      if (racl1->mask != NO_ENTRY && racl1->group_obj != racl2->group_obj)
++      if (racl1->mask != ACL_NO_ENTRY && racl1->group_obj != racl2->group_obj)
 +              return False;
 +      return ida_lists_equal(&racl1->users, &racl2->users)
 +          && ida_lists_equal(&racl1->groups, &racl2->groups);
@@ -389,23 +401,23 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +
 +static void send_rsync_acl(int f, const rsync_acl *racl)
 +{
-+      size_t count = rsync_acl_count_entries(racl);
++      size_t count = count_racl_entries(racl);
 +      write_int(f, count);
-+      if (racl->user_obj != NO_ENTRY) {
++      if (racl->user_obj != ACL_NO_ENTRY) {
 +              write_byte(f, 'u');
 +              write_byte(f, racl->user_obj);
 +      }
 +      send_ida_list(f, &racl->users, 'U');
-+      if (racl->group_obj != NO_ENTRY) {
++      if (racl->group_obj != ACL_NO_ENTRY) {
 +              write_byte(f, 'g');
 +              write_byte(f, racl->group_obj);
 +      }
 +      send_ida_list(f, &racl->groups, 'G');
-+      if (racl->mask != NO_ENTRY) {
++      if (racl->mask != ACL_NO_ENTRY) {
 +              write_byte(f, 'm');
 +              write_byte(f, racl->mask);
 +      }
-+      if (racl->other != NO_ENTRY) {
++      if (racl->other != ACL_NO_ENTRY) {
 +              write_byte(f, 'o');
 +              write_byte(f, racl->other);
 +      }
@@ -440,12 +452,12 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +                      sys_acl_free_acl(sacl);
 +                      if (!ok)
 +                              return -1;
-+#ifdef ACLS_NEED_MASK
-+                      /* Don't ever send a redundant mask value. */
-+                      if (!racl->users.count && !racl->groups.count
-+                       && racl->group == racl->mask)
-+                              racl->mask = NO_ENTRY;
-+#endif
++                      /* Avoid sending a redundant group/mask value. */
++                      if (curr_racl->group_obj == curr_racl->mask
++                       && (preserve_acls == 1
++                        || (!curr_racl->users.count
++                         && !curr_racl->groups.count)))
++                              curr_racl->mask = ACL_NO_ENTRY;
 +                      /* Strip access ACLs of permission-bit entries. */
 +                      if (type == SMB_ACL_TYPE_ACCESS && preserve_acls == 1)
 +                              rsync_acl_strip_perms(curr_racl);
@@ -635,7 +647,7 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +      const char *errfun = NULL;
 +      SMB_ACL_ENTRY_T entry;
 +
-+      if (!(*smb_acl = sys_acl_init(rsync_acl_count_entries(racl)))) {
++      if (!(*smb_acl = sys_acl_init(calc_sacl_entries(racl)))) {
 +              rprintf(FERROR, "pack_smb_acl: sys_acl_init(): %s\n",
 +                      strerror(errno));
 +              return False;
@@ -643,7 +655,7 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +
 +      COE( sys_acl_create_entry,(smb_acl, &entry) );
 +      COE( sys_acl_set_tag_type,(entry, SMB_ACL_USER_OBJ) );
-+      COE2( store_access_in_entry,(racl->user_obj, entry) );
++      COE2( store_access_in_entry,(racl->user_obj & 7, entry) );
 +
 +      for (ida = racl->users.idas, count = racl->users.count;
 +           count--; ida++) {
@@ -655,7 +667,7 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +
 +      COE( sys_acl_create_entry,(smb_acl, &entry) );
 +      COE( sys_acl_set_tag_type,(entry, SMB_ACL_GROUP_OBJ) );
-+      COE2( store_access_in_entry,(racl->group_obj, entry) );
++      COE2( store_access_in_entry,(racl->group_obj & 7, entry) );
 +
 +      for (ida = racl->groups.idas, count = racl->groups.count;
 +           count--; ida++) {
@@ -664,15 +676,19 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +              COE( sys_acl_set_qualifier,(entry, (void*)&ida->id) );
 +              COE2( store_access_in_entry,(ida->access, entry) );
 +      }
-+      if (racl->mask != NO_ENTRY) {
++#ifndef ACLS_NEED_MASK
++      if (racl->mask != ACL_NO_ENTRY) {
++#endif
 +              COE( sys_acl_create_entry,(smb_acl, &entry) );
 +              COE( sys_acl_set_tag_type,(entry, SMB_ACL_MASK) );
 +              COE2( store_access_in_entry,(racl->mask, entry) );
++#ifndef ACLS_NEED_MASK
 +      }
++#endif
 +
 +      COE( sys_acl_create_entry,(smb_acl, &entry) );
 +      COE( sys_acl_set_tag_type,(entry, SMB_ACL_OTHER) );
-+      COE2( store_access_in_entry,(racl->other, entry) );
++      COE2( store_access_in_entry,(racl->other & 7, entry) );
 +
 +#ifdef DEBUG
 +      if (sys_acl_valid(*smb_acl) < 0)
@@ -690,10 +706,9 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +      return False;
 +}
 +
-+static mode_t change_sacl_perms(SMB_ACL_T sacl, uchar mask, mode_t old_mode, mode_t mode)
++static mode_t change_sacl_perms(SMB_ACL_T sacl, rsync_acl *racl, mode_t old_mode, mode_t mode)
 +{
 +      SMB_ACL_ENTRY_T entry;
-+      int group_id = mask != NO_ENTRY ? SMB_ACL_MASK : SMB_ACL_GROUP_OBJ;
 +      const char *errfun;
 +      int rc;
 +
@@ -724,12 +739,28 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +                      errfun = "sys_acl_get_tag_type";
 +                      break;
 +              }
-+              if (tag_type == SMB_ACL_USER_OBJ)
++              switch (tag_type) {
++              case SMB_ACL_USER_OBJ:
 +                      COE2( store_access_in_entry,((mode >> 6) & 7, entry) );
-+              else if (tag_type == group_id)
++                      break;
++              case SMB_ACL_GROUP_OBJ:
++                      /* group is only empty when identical to group perms. */
++                      if (racl->group_obj != ACL_NO_ENTRY)
++                              break;
 +                      COE2( store_access_in_entry,((mode >> 3) & 7, entry) );
-+              else if (tag_type == SMB_ACL_OTHER)
++                      break;
++              case SMB_ACL_MASK:
++#ifndef ACLS_NEED_MASK
++                      /* mask is only empty when we don't need it. */
++                      if (racl->mask == ACL_NO_ENTRY)
++                              break;
++#endif
++                      COE2( store_access_in_entry,((mode >> 3) & 7, entry) );
++                      break;
++              case SMB_ACL_OTHER:
 +                      COE2( store_access_in_entry,(mode & 7, entry) );
++                      break;
++              }
 +      }
 +      if (rc) {
 +        error_exit:
@@ -751,7 +782,7 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +      return (old_mode & ~ACCESSPERMS) | (mode & ACCESSPERMS);
 +}
 +
-+static void receive_rsync_acl(rsync_acl *racl, int f)
++static void receive_rsync_acl(rsync_acl *racl, int f, SMB_ACL_TYPE_T type)
 +{
 +      uchar computed_mask_bits = 0;
 +      ida_list *idal = NULL;
@@ -773,7 +804,7 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +              }
 +              switch (tag) {
 +              case 'u':
-+                      if (racl->user_obj != NO_ENTRY) {
++                      if (racl->user_obj != ACL_NO_ENTRY) {
 +                              rprintf(FERROR, "receive_rsync_acl: error: duplicate USER_OBJ entry\n");
 +                              exit_cleanup(RERR_STREAMIO);
 +                      }
@@ -783,7 +814,7 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +                      idal = &racl->users;
 +                      break;
 +              case 'g':
-+                      if (racl->group_obj != NO_ENTRY) {
++                      if (racl->group_obj != ACL_NO_ENTRY) {
 +                              rprintf(FERROR, "receive_rsync_acl: error: duplicate GROUP_OBJ entry\n");
 +                              exit_cleanup(RERR_STREAMIO);
 +                      }
@@ -793,14 +824,14 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +                      idal = &racl->groups;
 +                      break;
 +              case 'm':
-+                      if (racl->mask != NO_ENTRY) {
++                      if (racl->mask != ACL_NO_ENTRY) {
 +                              rprintf(FERROR, "receive_rsync_acl: error: duplicate MASK entry\n");
 +                              exit_cleanup(RERR_STREAMIO);
 +                      }
 +                      racl->mask = access;
 +                      continue;
 +              case 'o':
-+                      if (racl->other != NO_ENTRY) {
++                      if (racl->other != ACL_NO_ENTRY) {
 +                              rprintf(FERROR, "receive_rsync_acl: error: duplicate OTHER entry\n");
 +                              exit_cleanup(RERR_STREAMIO);
 +                      }
@@ -818,26 +849,28 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +              computed_mask_bits |= access;
 +      }
 +
-+      /* Ensure that these are never unset. */
-+      if (racl->user_obj == NO_ENTRY)
-+              racl->user_obj = 7;
-+      if (racl->group_obj == NO_ENTRY)
-+              racl->group_obj = 0;
-+      if (racl->other == NO_ENTRY)
-+              racl->other = 0;
++      if (type == SMB_ACL_TYPE_DEFAULT) {
++              /* Ensure that these are never unset. */
++              if (racl->user_obj == ACL_NO_ENTRY)
++                      racl->user_obj = 7;
++              if (racl->group_obj == ACL_NO_ENTRY)
++                      racl->group_obj = 0;
++              if (racl->other == ACL_NO_ENTRY)
++                      racl->other = 0;
++      }
 +#ifndef ACLS_NEED_MASK
 +      if (!racl->users.count && !racl->groups.count) {
 +              /* If we, a system without ACLS_NEED_MASK, received a
 +               * superfluous mask, throw it away. */
-+              if (racl->mask != NO_ENTRY) {
++              if (racl->mask != ACL_NO_ENTRY) {
 +                      /* mask off group perms with it first */
-+                      racl->group_obj &= racl->mask;
-+                      racl->mask = NO_ENTRY;
++                      racl->group_obj &= racl->mask | ACL_NO_ENTRY;
++                      racl->mask = ACL_NO_ENTRY;
 +              }
 +      } else
 +#endif
-+      if (racl->mask == NO_ENTRY)
-+              racl->mask = computed_mask_bits | racl->group_obj;
++      if (racl->mask == ACL_NO_ENTRY) /* always made non-empty when needed */
++              racl->mask = computed_mask_bits | (racl->group_obj & 7);
 +}
 +
 +/* receive and build the rsync_acl_lists */
@@ -883,7 +916,7 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +                              aclidx = racl_list->count;
 +                      fileaclidx_list->fileaclidxs[fileaclidx_list->count++].
 +                              file = file;
-+                      receive_rsync_acl(&racl, f);
++                      receive_rsync_acl(&racl, f, type);
 +                      expand_rsync_acl_list(racl_list);
 +                      racl_list->racls[racl_list->count++] = racl;
 +                      expand_smb_acl_list(sacl_list);
@@ -994,7 +1027,7 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +                      ; /* presume they're unequal */
 +              }
 +              if (type == SMB_ACL_TYPE_DEFAULT
-+               && rsync_acl_count_entries(&racl_orig) == 0) {
++               && racl_orig.user_obj == ACL_NO_ENTRY) {
 +                      if (sys_acl_delete_def_file(bak) < 0) {
 +                              rprintf(FERROR, "dup_acl: sys_acl_delete_def_file(%s): %s\n",
 +                                      bak, strerror(errno));
@@ -1148,7 +1181,7 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +                      continue;
 +              if (!dry_run && mode_p) {
 +                      if (type == SMB_ACL_TYPE_DEFAULT
-+                       && rsync_acl_count_entries(racl_new) == 0) {
++                       && racl_new->user_obj == ACL_NO_ENTRY) {
 +                              if (sys_acl_delete_def_file(fname) < 0) {
 +                                      rprintf(FERROR, "set_acl: sys_acl_delete_def_file(%s): %s\n",
 +                                              fname, strerror(errno));
@@ -1163,7 +1196,7 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +                                      continue;
 +                              }
 +                              if (type == SMB_ACL_TYPE_ACCESS) {
-+                                      cur_mode = change_sacl_perms(*sacl_new, racl_new->mask,
++                                      cur_mode = change_sacl_perms(*sacl_new, racl_new,
 +                                                                   cur_mode, file->mode);
 +                                      if (cur_mode == ~0u)
 +                                              continue;
@@ -1286,7 +1319,7 @@ This code does not yet itemize changes in ACL information (see --itemize).
 +      }
 +
 +      /* Apply the permission-bit entries of the default ACL, if any. */
-+      if (rsync_acl_count_entries(&racl) > 0) {
++      if (racl.user_obj != ACL_NO_ENTRY) {
 +              perms = rsync_acl_get_perms(&racl);
 +              if (verbose > 2)
 +                      rprintf(FINFO, "got ACL-based default perms %o for directory %s\n", perms, dir);