rgw/acl: add_grant() takes const ref

that also required fixing some const-correctness issues

Signed-off-by: Casey Bodley <cbodley@redhat.com>

diff --git a/src/rgw/driver/rados/rgw_sal_rados.cc b/src/rgw/driver/rados/rgw_sal_rados.cc
index 94c6001983ce822a757eae81d8d2da03f713c25b..5ede8d44fa9b1ae4d260dc589500d418aa3794db 100644 (file)
--- a/src/rgw/driver/rados/rgw_sal_rados.cc
+++ b/src/rgw/driver/rados/rgw_sal_rados.cc
@@ -1670,7 +1670,7 @@ int RadosObject::chown(User& new_user, const DoutPrefixProvider* dpp, optional_y
   //Create a grant and add grant
   ACLGrant grant;
   grant.set_canon(new_user.get_id(), new_user.get_display_name(), RGW_PERM_FULL_CONTROL);
-  acl.add_grant(&grant);
+  acl.add_grant(grant);
 
   //Update the ACL owner to the new user
   owner.id = new_user.get_id();

diff --git a/src/rgw/rgw_acl.cc b/src/rgw/rgw_acl.cc
index 18824f86fe94ab77d27db540153a630dc51f1cbb..6e3aafe762ad83c524e59aa0e0c69f5f49cb2b38 100644 (file)
--- a/src/rgw/rgw_acl.cc
+++ b/src/rgw/rgw_acl.cc
@@ -70,38 +70,38 @@ bool operator!=(const RGWAccessControlPolicy& lhs,
   return !(lhs == rhs);
 }
 
-void RGWAccessControlList::_add_grant(ACLGrant *grant)
+void RGWAccessControlList::register_grant(const ACLGrant& grant)
 {
-  ACLPermission& perm = grant->get_permission();
-  ACLGranteeType& type = grant->get_type();
+  ACLPermission perm = grant.get_permission();
+  ACLGranteeType type = grant.get_type();
   switch (type.get_type()) {
   case ACL_TYPE_REFERER:
-    referer_list.emplace_back(grant->get_referer(), perm.get_permissions());
+    referer_list.emplace_back(grant.get_referer(), perm.get_permissions());
 
     /* We're specially handling the Swift's .r:* as the S3 API has a similar
      * concept and thus we can have a small portion of compatibility here. */
-     if (grant->get_referer() == RGW_REFERER_WILDCARD) {
+     if (grant.get_referer() == RGW_REFERER_WILDCARD) {
        acl_group_map[ACL_GROUP_ALL_USERS] |= perm.get_permissions();
      }
     break;
   case ACL_TYPE_GROUP:
-    acl_group_map[grant->get_group()] |= perm.get_permissions();
+    acl_group_map[grant.get_group()] |= perm.get_permissions();
     break;
   default:
     {
       rgw_user id;
-      grant->get_id(id);
+      grant.get_id(id);
       acl_user_map[id.to_str()] |= perm.get_permissions();
     }
   }
 }
 
-void RGWAccessControlList::add_grant(ACLGrant *grant)
+void RGWAccessControlList::add_grant(const ACLGrant& grant)
 {
   rgw_user id;
-  grant->get_id(id); // not that this will return false for groups, but that's ok, we won't search groups
-  grant_map.insert(pair<string, ACLGrant>(id.to_str(), *grant));
-  _add_grant(grant);
+  grant.get_id(id); // note that this will return false for groups, but that's ok, we won't search groups
+  grant_map.emplace(id.to_str(), grant);
+  register_grant(grant);
 }
 
 void RGWAccessControlList::remove_canon_user_grant(rgw_user& user_id)
@@ -323,14 +323,10 @@ void RGWAccessControlList::generate_test_instances(list<RGWAccessControlList*>&
 {
   RGWAccessControlList *acl = new RGWAccessControlList;
 
-  list<ACLGrant *> glist;
-  list<ACLGrant *>::iterator iter;
-
-  ACLGrant::generate_test_instances(glist);
-  for (iter = glist.begin(); iter != glist.end(); ++iter) {
-    ACLGrant *grant = *iter;
-    acl->add_grant(grant);
-
+  list<ACLGrant *> grants;
+  ACLGrant::generate_test_instances(grants);
+  for (ACLGrant* grant : grants) {
+    acl->add_grant(*grant);
     delete grant;
   }
   o.push_back(acl);

diff --git a/src/rgw/rgw_acl.h b/src/rgw/rgw_acl.h
index 13141279e8df376f820b126217e870c71d49de1f..f60295d90ab4ce155a188d1bb7b5b9fa93d15b5a 100644 (file)
--- a/src/rgw/rgw_acl.h
+++ b/src/rgw/rgw_acl.h
@@ -60,10 +60,8 @@ public:
     }
   }
 
-  ACLGranteeType& get_type() { return type; }
-  const ACLGranteeType& get_type() const { return type; }
-  ACLPermission& get_permission() { return permission; }
-  const ACLPermission& get_permission() const { return permission; }
+  ACLGranteeType get_type() const { return type; }
+  ACLPermission get_permission() const { return permission; }
   ACLGroupTypeEnum get_group() const { return group; }
   const std::string& get_referer() const { return url_spec; }
 
@@ -225,7 +223,8 @@ protected:
   std::map<uint32_t, int> acl_group_map;
   std::list<ACLReferer> referer_list;
   ACLGrantMap grant_map;
-  void _add_grant(ACLGrant *grant);
+  // register a grant in the correspoding acl_user/group_map
+  void register_grant(const ACLGrant& grant);
 public:
   uint32_t get_perm(const DoutPrefixProvider* dpp,
                     const rgw::auth::Identity& auth_identity,
@@ -253,10 +252,9 @@ public:
     if (struct_v >= 2) {
       decode(acl_group_map, bl);
     } else if (!maps_initialized) {
-      ACLGrantMap::iterator iter;
-      for (iter = grant_map.begin(); iter != grant_map.end(); ++iter) {
-        ACLGrant& grant = iter->second;
-        _add_grant(&grant);
+      // register everything in the grant_map
+      for (const auto& [id, grant] : grant_map) {
+        register_grant(grant);
       }
     }
     if (struct_v >= 4) {
@@ -267,7 +265,7 @@ public:
   void dump(Formatter *f) const;
   static void generate_test_instances(std::list<RGWAccessControlList*>& o);
 
-  void add_grant(ACLGrant *grant);
+  void add_grant(const ACLGrant& grant);
   void remove_canon_user_grant(rgw_user& user_id);
 
   ACLGrantMap& get_grant_map() { return grant_map; }
@@ -280,7 +278,7 @@ public:
 
     ACLGrant grant;
     grant.set_canon(id, name, RGW_PERM_FULL_CONTROL);
-    add_grant(&grant);
+    add_grant(grant);
   }
 
   friend bool operator==(const RGWAccessControlList& lhs, const RGWAccessControlList& rhs);

diff --git a/src/rgw/rgw_acl_s3.cc b/src/rgw/rgw_acl_s3.cc
index 2641c19ec178de54c54369e624b3f83265466e25..52eeb44f2430e2cdd96affbef03203228f689fc8 100644 (file)
--- a/src/rgw/rgw_acl_s3.cc
+++ b/src/rgw/rgw_acl_s3.cc
@@ -66,7 +66,7 @@ xml_end(const char *el)
 
 class ACLGranteeType_S3 {
 public:
-  static const char *to_string(ACLGranteeType& type) {
+  static const char *to_string(ACLGranteeType type) {
     switch (type.get_type()) {
     case ACL_TYPE_CANON_USER:
       return "CanonicalUser";
@@ -264,7 +264,7 @@ bool RGWAccessControlList_S3::xml_end(const char *el) {
   XMLObjIter iter = find("Grant");
   ACLGrant_S3 *grant = static_cast<ACLGrant_S3 *>(iter.get_next());
   while (grant) {
-    add_grant(grant);
+    add_grant(*grant);
     grant = static_cast<ACLGrant_S3 *>(iter.get_next());
   }
   return true;
@@ -371,7 +371,7 @@ int RGWAccessControlList_S3::create_canned(ACLOwner& owner, ACLOwner& bucket_own
 
   /* owner gets full control */
   owner_grant.set_canon(owner.id, owner.display_name, RGW_PERM_FULL_CONTROL);
-  add_grant(&owner_grant);
+  add_grant(owner_grant);
 
   if (canned_acl.size() == 0 || canned_acl.compare("private") == 0) {
     return 0;
@@ -381,24 +381,24 @@ int RGWAccessControlList_S3::create_canned(ACLOwner& owner, ACLOwner& bucket_own
   ACLGrant group_grant;
   if (canned_acl.compare("public-read") == 0) {
     group_grant.set_group(ACL_GROUP_ALL_USERS, RGW_PERM_READ);
-    add_grant(&group_grant);
+    add_grant(group_grant);
   } else if (canned_acl.compare("public-read-write") == 0) {
     group_grant.set_group(ACL_GROUP_ALL_USERS, RGW_PERM_READ);
-    add_grant(&group_grant);
+    add_grant(group_grant);
     group_grant.set_group(ACL_GROUP_ALL_USERS, RGW_PERM_WRITE);
-    add_grant(&group_grant);
+    add_grant(group_grant);
   } else if (canned_acl.compare("authenticated-read") == 0) {
     group_grant.set_group(ACL_GROUP_AUTHENTICATED_USERS, RGW_PERM_READ);
-    add_grant(&group_grant);
+    add_grant(group_grant);
   } else if (canned_acl.compare("bucket-owner-read") == 0) {
     bucket_owner_grant.set_canon(bid, bname, RGW_PERM_READ);
     if (bid != owner.id) {
-      add_grant(&bucket_owner_grant);
+      add_grant(bucket_owner_grant);
     }
   } else if (canned_acl.compare("bucket-owner-full-control") == 0) {
     bucket_owner_grant.set_canon(bid, bname, RGW_PERM_FULL_CONTROL);
     if (bid != owner.id) {
-      add_grant(&bucket_owner_grant);
+      add_grant(bucket_owner_grant);
     }
   } else {
     return -EINVAL;
@@ -415,9 +415,8 @@ int RGWAccessControlList_S3::create_from_grants(std::list<ACLGrant>& grants)
   acl_user_map.clear();
   grant_map.clear();
 
-  for (std::list<ACLGrant>::iterator it = grants.begin(); it != grants.end(); ++it) {
-    ACLGrant g = *it;
-    add_grant(&g);
+  for (const auto& g : grants) {
+    add_grant(g);
   }
 
   return 0;
@@ -513,7 +512,7 @@ int RGWAccessControlPolicy_S3::rebuild(const DoutPrefixProvider *dpp,
   multimap<string, ACLGrant>::iterator iter;
   for (iter = grant_map.begin(); iter != grant_map.end(); ++iter) {
     ACLGrant& src_grant = iter->second;
-    ACLGranteeType& type = src_grant.get_type();
+    ACLGranteeType type = src_grant.get_type();
     ACLGrant new_grant;
     bool grant_ok = false;
     rgw_user uid;
@@ -557,7 +556,7 @@ int RGWAccessControlPolicy_S3::rebuild(const DoutPrefixProvider *dpp,
            grant_user = user->get_info();
          }
         }
-       ACLPermission& perm = src_grant.get_permission();
+       ACLPermission perm = src_grant.get_permission();
        new_grant.set_canon(uid, grant_user.display_name, perm.get_permissions());
        grant_ok = true;
        rgw_user new_id;
@@ -582,7 +581,7 @@ int RGWAccessControlPolicy_S3::rebuild(const DoutPrefixProvider *dpp,
       break;
     }
     if (grant_ok) {
-      dst_acl.add_grant(&new_grant);
+      dst_acl.add_grant(new_grant);
     }
   }
 

diff --git a/src/rgw/rgw_acl_swift.cc b/src/rgw/rgw_acl_swift.cc
index e829788c6d5a955baa52f732a9ec1d7e3dd3f7ed..70fa793dd9af9d4ebd604baf765419995702f744 100644 (file)
--- a/src/rgw/rgw_acl_swift.cc
+++ b/src/rgw/rgw_acl_swift.cc
@@ -168,7 +168,7 @@ int RGWAccessControlPolicy_SWIFT::add_grants(const DoutPrefixProvider *dpp,
     }
 
     if (grant) {
-      acl.add_grant(&*grant);
+      acl.add_grant(*grant);
     } else {
       return -EINVAL;
     }
@@ -257,7 +257,7 @@ void RGWAccessControlPolicy_SWIFT::filter_merge(uint32_t rw_mask,
       }
     }
     if (perm & rw_mask) {
-      acl.add_grant(&grant);
+      acl.add_grant(grant);
     }
   }
 }
@@ -313,7 +313,7 @@ void RGWAccessControlPolicy_SWIFTAcct::add_grants(const DoutPrefixProvider *dpp,
 
     if (uid_is_public(uid)) {
       grant.set_group(ACL_GROUP_ALL_USERS, perm);
-      acl.add_grant(&grant);
+      acl.add_grant(grant);
     } else  {
       std::unique_ptr<rgw::sal::User> user = driver->get_user(rgw_user(uid));
 
@@ -321,10 +321,10 @@ void RGWAccessControlPolicy_SWIFTAcct::add_grants(const DoutPrefixProvider *dpp,
         ldpp_dout(dpp, 10) << "grant user does not exist:" << uid << dendl;
         /* skipping silently */
         grant.set_canon(user->get_id(), std::string(), perm);
-        acl.add_grant(&grant);
+        acl.add_grant(grant);
       } else {
         grant.set_canon(user->get_id(), user->get_display_name(), perm);
-        acl.add_grant(&grant);
+        acl.add_grant(grant);
       }
     }
   }

diff --git a/src/rgw/rgw_rest_client.cc b/src/rgw/rgw_rest_client.cc
index 1ccb813a1098adc85a11b3abfc93e78d1f4c1cc9..71ead6a8b8923d84fbee52d1b2934ad7043c0f61 100644 (file)
--- a/src/rgw/rgw_rest_client.cc
+++ b/src/rgw/rgw_rest_client.cc
@@ -497,7 +497,7 @@ RGWRESTStreamS3PutObj::~RGWRESTStreamS3PutObj()
   delete out_cb;
 }
 
-static void grants_by_type_add_one_grant(map<int, string>& grants_by_type, int perm, ACLGrant& grant)
+static void grants_by_type_add_one_grant(map<int, string>& grants_by_type, int perm, const ACLGrant& grant)
 {
   string& s = grants_by_type[perm];
 
@@ -505,7 +505,7 @@ static void grants_by_type_add_one_grant(map<int, string>& grants_by_type, int p
     s.append(", ");
 
   string id_type_str;
-  ACLGranteeType& type = grant.get_type();
+  ACLGranteeType type = grant.get_type();
   switch (type.get_type()) {
     case ACL_TYPE_GROUP:
       id_type_str = "uri";
@@ -535,7 +535,7 @@ struct grant_type_to_header grants_headers_def[] = {
   { 0, NULL}
 };
 
-static bool grants_by_type_check_perm(map<int, string>& grants_by_type, int perm, ACLGrant& grant, int check_perm)
+static bool grants_by_type_check_perm(map<int, string>& grants_by_type, int perm, const ACLGrant& grant, int check_perm)
 {
   if ((perm & check_perm) == check_perm) {
     grants_by_type_add_one_grant(grants_by_type, check_perm, grant);
@@ -544,7 +544,7 @@ static bool grants_by_type_check_perm(map<int, string>& grants_by_type, int perm
   return false;
 }
 
-static void grants_by_type_add_perm(map<int, string>& grants_by_type, int perm, ACLGrant& grant)
+static void grants_by_type_add_perm(map<int, string>& grants_by_type, int perm, const ACLGrant& grant)
 {
   struct grant_type_to_header *t;
 
@@ -669,16 +669,13 @@ void RGWRESTGenerateHTTPHeaders::set_http_attrs(const map<string, string>& http_
   }
 }
 
-void RGWRESTGenerateHTTPHeaders::set_policy(RGWAccessControlPolicy& policy)
+void RGWRESTGenerateHTTPHeaders::set_policy(const RGWAccessControlPolicy& policy)
 {
   /* update acl headers */
-  RGWAccessControlList& acl = policy.get_acl();
-  multimap<string, ACLGrant>& grant_map = acl.get_grant_map();
-  multimap<string, ACLGrant>::iterator giter;
+  const RGWAccessControlList& acl = policy.get_acl();
   map<int, string> grants_by_type;
-  for (giter = grant_map.begin(); giter != grant_map.end(); ++giter) {
-    ACLGrant& grant = giter->second;
-    ACLPermission& perm = grant.get_permission();
+  for (const auto& [id, grant] : acl.get_grant_map()) {
+    ACLPermission perm = grant.get_permission();
     grants_by_type_add_perm(grants_by_type, perm.get_permissions(), grant);
   }
   add_grants_headers(grants_by_type, *new_env, new_info->x_meta_map);

diff --git a/src/rgw/rgw_rest_client.h b/src/rgw/rgw_rest_client.h
index 6e27576c6768fd5f54f82ad23df70fdc9b086e11..923f8cc17839b00d6065306dfed88f44098501cb 100644 (file)
--- a/src/rgw/rgw_rest_client.h
+++ b/src/rgw/rgw_rest_client.h
@@ -94,7 +94,7 @@ public:
   void set_extra_headers(const std::map<std::string, std::string>& extra_headers);
   int set_obj_attrs(const DoutPrefixProvider *dpp, std::map<std::string, bufferlist>& rgw_attrs);
   void set_http_attrs(const std::map<std::string, std::string>& http_attrs);
-  void set_policy(RGWAccessControlPolicy& policy);
+  void set_policy(const RGWAccessControlPolicy& policy);
   int sign(const DoutPrefixProvider *dpp, RGWAccessKey& key, const bufferlist *opt_content);
 
   const std::string& get_url() { return url; }