rgw: make RGWEnv return a const ref. to its map

We already have a public method `set` for setting values in the RGW env
map, making the map as a read only. In addition:

- Added const to other  methods in `RGWEnv` which are getters
- `req_info::init_meta` also reused the same `iter` name for an internal
  find, changed the var name
- `add_grants_headers` now accepts an RGWEnv instead of the map
- use range based for loops wherever the code was changed
- req_info holds a ptr to a const RGWEnv, since we seem to only read the
  values from it

Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

diff --git a/src/rgw/rgw_acl_s3.cc b/src/rgw/rgw_acl_s3.cc
index 0eb3a78a0e24a9a07dd9da003263901bcc098e5f..eec7db6898a6a9226013fdb1cec166a8db9e21b5 100644 (file)
--- a/src/rgw/rgw_acl_s3.cc
+++ b/src/rgw/rgw_acl_s3.cc
@@ -283,7 +283,7 @@ struct s3_acl_header {
   const char *http_header;
 };
 
-static const char *get_acl_header(RGWEnv *env,
+static const char *get_acl_header(const RGWEnv *env,
         const struct s3_acl_header *perm)
 {
   const char *header = perm->http_header;
@@ -332,7 +332,7 @@ static int parse_grantee_str(RGWRados *store, string& grantee_str,
   return 0;
 }
 
-static int parse_acl_header(RGWRados *store, RGWEnv *env,
+static int parse_acl_header(RGWRados *store, const RGWEnv *env,
          const struct s3_acl_header *perm, std::list<ACLGrant>& _grants)
 {
   std::list<string> grantees;
@@ -452,7 +452,7 @@ static const s3_acl_header acl_header_perms[] = {
   {0, NULL}
 };
 
-int RGWAccessControlPolicy_S3::create_from_headers(RGWRados *store, RGWEnv *env, ACLOwner& _owner)
+int RGWAccessControlPolicy_S3::create_from_headers(RGWRados *store, const RGWEnv *env, ACLOwner& _owner)
 {
   std::list<ACLGrant> grants;
 

diff --git a/src/rgw/rgw_acl_s3.h b/src/rgw/rgw_acl_s3.h
index 6991a0b83639ac2d4dfe35f5679f0a0358479509..149db40e70b1eaa9f6ec0845c610f572860a172b 100644 (file)
--- a/src/rgw/rgw_acl_s3.h
+++ b/src/rgw/rgw_acl_s3.h
@@ -92,7 +92,7 @@ public:
     owner = _owner;
     return ret;
   }
-  int create_from_headers(RGWRados *store, RGWEnv *env, ACLOwner& _owner);
+  int create_from_headers(RGWRados *store, const RGWEnv *env, ACLOwner& _owner);
 };
 
 /**

diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc
index 7b320b804887c9f3649467c2ff77ffbbd0c33bd6..8694805d59c207868715c4b644de1601968f377b 100644 (file)
--- a/src/rgw/rgw_common.cc
+++ b/src/rgw/rgw_common.cc
@@ -203,7 +203,7 @@ static string get_abs_path(const string& request_uri) {
   return request_uri.substr(beg_pos, len - beg_pos);
 }
 
-req_info::req_info(CephContext *cct, class RGWEnv *e) : env(e) {
+req_info::req_info(CephContext *cct, const class RGWEnv *env) : env(env) {
   method = env->get("REQUEST_METHOD", "");
   script_uri = env->get("SCRIPT_URI", cct->_conf->rgw_script_uri.c_str());
   request_uri = env->get("REQUEST_URI", cct->_conf->rgw_request_uri.c_str());
@@ -383,12 +383,10 @@ void req_info::init_meta_info(bool *found_bad_meta)
 {
   x_meta_map.clear();
 
-  map<string, string, ltstr_nocase>& m = env->get_map();
-  map<string, string, ltstr_nocase>::iterator iter;
-  for (iter = m.begin(); iter != m.end(); ++iter) {
+  for (const auto& kv: env->get_map()) {
     const char *prefix;
-    const string& header_name = iter->first;
-    const string& val = iter->second;
+    const string& header_name = kv.first;
+    const string& val = kv.second;
     for (int prefix_num = 0; (prefix = meta_prefixes[prefix_num].str) != NULL; prefix_num++) {
       int len = meta_prefixes[prefix_num].len;
       const char *p = header_name.c_str();
@@ -411,12 +409,10 @@ void req_info::init_meta_info(bool *found_bad_meta)
         }
         name_low[j] = 0;
 
-        map<string, string>::iterator iter;
-        iter = x_meta_map.find(name_low);
-        if (iter != x_meta_map.end()) {
-          string old = iter->second;
-          int pos = old.find_last_not_of(" \t"); /* get rid of any whitespaces after the value */
-          old = old.substr(0, pos + 1);
+        auto it = x_meta_map.find(name_low);
+        if (it != x_meta_map.end()) {
+          string old = it->second;
+          boost::algorithm::trim_right(old);
           old.append(",");
           old.append(val);
           x_meta_map[name_low] = old;
@@ -426,8 +422,8 @@ void req_info::init_meta_info(bool *found_bad_meta)
       }
     }
   }
-  for (iter = x_meta_map.begin(); iter != x_meta_map.end(); ++iter) {
-    dout(10) << "x>> " << iter->first << ":" << rgw::crypt_sanitize::x_meta_map{iter->first, iter->second} << dendl;
+  for (const auto& kv: x_meta_map) {
+    dout(10) << "x>> " << kv.first << ":" << rgw::crypt_sanitize::x_meta_map{kv.first, kv.second} << dendl;
   }
 }
 

diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h
index af3d7c15bb711b20057c5c1106af2a6457cc4ea7..3b5c115fb605a18fbd5ae0d8bdf2a0a7b6c0e0ba 100644 (file)
--- a/src/rgw/rgw_common.h
+++ b/src/rgw/rgw_common.h
@@ -393,16 +393,16 @@ public:
   void init(CephContext *cct);
   void init(CephContext *cct, char **envp);
   void set(const boost::string_ref& name, const boost::string_ref& val);
-  const char *get(const char *name, const char *def_val = NULL);
-  int get_int(const char *name, int def_val = 0);
+  const char *get(const char *name, const char *def_val = nullptr) const;
+  int get_int(const char *name, int def_val = 0) const;
   bool get_bool(const char *name, bool def_val = 0);
-  size_t get_size(const char *name, size_t def_val = 0);
-  bool exists(const char *name);
-  bool exists_prefix(const char *prefix);
+  size_t get_size(const char *name, size_t def_val = 0) const;
+  bool exists(const char *name) const;
+  bool exists_prefix(const char *prefix) const;
 
   void remove(const char *name);
 
-  std::map<string, string, ltstr_nocase>& get_map() { return env_map; }
+  const std::map<string, string, ltstr_nocase>& get_map() const { return env_map; }
 };
 
 enum http_op {
@@ -1397,7 +1397,7 @@ namespace rgw {
 
 
 struct req_info {
-  RGWEnv *env;
+  const RGWEnv *env;
   RGWHTTPArgs args;
   map<string, string> x_meta_map;
 
@@ -1410,7 +1410,7 @@ struct req_info {
   string request_params;
   string domain;
 
-  req_info(CephContext *cct, RGWEnv *_env);
+  req_info(CephContext *cct, const RGWEnv *env);
   void rebuild_from(req_info& src);
   void init_meta_info(bool *found_bad_meta);
 };

diff --git a/src/rgw/rgw_crypt.cc b/src/rgw/rgw_crypt.cc
index 9fb474c46f0ecf90263f96e41e7c4e0bd3f02e36..d2f5a443657eb745ae3662974fec718c4681d06b 100644 (file)
--- a/src/rgw/rgw_crypt.cc
+++ b/src/rgw/rgw_crypt.cc
@@ -1052,7 +1052,7 @@ static const crypt_option_names crypt_options[] = {
 };
 
 static boost::string_view get_crypt_attribute(
-    RGWEnv* env,
+    const RGWEnv* env,
     std::map<std::string,
              RGWPostObj_ObjStore::post_form_part,
              const ltstr_nocase>* parts,

diff --git a/src/rgw/rgw_env.cc b/src/rgw/rgw_env.cc
index d1f876dcbee1a3bdd247c3961ac9553946255a32..bc195f71e76af405d333b46580d6ca747e10b9e9 100644 (file)
--- a/src/rgw/rgw_env.cc
+++ b/src/rgw/rgw_env.cc
@@ -50,7 +50,7 @@ const char *rgw_conf_get(const map<string, string, ltstr_nocase>& conf_map, cons
   return iter->second.c_str();
 }
 
-const char *RGWEnv::get(const char *name, const char *def_val)
+const char *RGWEnv::get(const char *name, const char *def_val) const
 {
   return rgw_conf_get(env_map, name, def_val);
 }
@@ -65,7 +65,7 @@ int rgw_conf_get_int(const map<string, string, ltstr_nocase>& conf_map, const ch
   return atoi(s);  
 }
 
-int RGWEnv::get_int(const char *name, int def_val)
+int RGWEnv::get_int(const char *name, int def_val) const
 {
   return rgw_conf_get_int(env_map, name, def_val);
 }
@@ -85,28 +85,35 @@ bool RGWEnv::get_bool(const char *name, bool def_val)
   return rgw_conf_get_bool(env_map, name, def_val);
 }
 
-size_t RGWEnv::get_size(const char *name, size_t def_val)
+size_t RGWEnv::get_size(const char *name, size_t def_val) const
 {
-  map<string, string, ltstr_nocase>::iterator iter = env_map.find(name);
+  const auto iter = env_map.find(name);
   if (iter == env_map.end())
     return def_val;
 
-  const char *s = iter->second.c_str();
-  return atoll(s);  
+  size_t sz;
+  try{
+    sz = stoull(iter->second);
+  } catch(...){
+    /* it is very unlikely that we'll ever encounter out_of_range, but let's
+       return the default eitherway */
+    sz = def_val;
+  }
+
+  return sz;
 }
 
-bool RGWEnv::exists(const char *name)
+bool RGWEnv::exists(const char *name) const
 {
-  map<string, string, ltstr_nocase>::iterator iter = env_map.find(name);
-  return (iter != env_map.end());
+  return env_map.find(name)!= env_map.end();
 }
 
-bool RGWEnv::exists_prefix(const char *prefix)
+bool RGWEnv::exists_prefix(const char *prefix) const
 {
   if (env_map.empty() || prefix == NULL)
     return false;
 
-  map<string, string, ltstr_nocase>::iterator iter = env_map.lower_bound(prefix);
+  const auto iter = env_map.lower_bound(prefix);
   if (iter == env_map.end())
     return false;
 

diff --git a/src/rgw/rgw_rest_client.cc b/src/rgw/rgw_rest_client.cc
index aa3de5fac8378fee5843cbd4acd67beafe53e5cc..f1ec7d72daeb4b0d5fe7a1b805240cc15f87615f 100644 (file)
--- a/src/rgw/rgw_rest_client.cc
+++ b/src/rgw/rgw_rest_client.cc
@@ -216,12 +216,9 @@ int RGWRESTSimpleRequest::sign_request(RGWAccessKey& key, RGWEnv& env, req_info&
     return 0;
   }
 
-  map<string, string, ltstr_nocase>& m = env.get_map();
-
   if (cct->_conf->subsys.should_gather(ceph_subsys_rgw, 20)) {
-    map<string, string>::iterator i;
-    for (i = m.begin(); i != m.end(); ++i) {
-      ldout(cct, 20) << "> " << i->first << " -> " << rgw::crypt_sanitize::x_meta_map{i->first, i->second} << dendl;
+    for (const auto& i: env.get_map()) {
+      ldout(cct, 20) << "> " << i.first << " -> " << rgw::crypt_sanitize::x_meta_map{i.first, i.second} << dendl;
     }
   }
 
@@ -243,7 +240,7 @@ int RGWRESTSimpleRequest::sign_request(RGWAccessKey& key, RGWEnv& env, req_info&
   string auth_hdr = "AWS " + key.id + ":" + digest;
   ldout(cct, 15) << "generated auth header: " << auth_hdr << dendl;
   
-  m["AUTHORIZATION"] = auth_hdr;
+  env.set("AUTHORIZATION", auth_hdr);
 
   return 0;
 }
@@ -266,15 +263,13 @@ int RGWRESTSimpleRequest::forward_request(RGWAccessKey& key, req_info& info, siz
     return ret;
   }
 
-  map<string, string, ltstr_nocase>& m = new_env.get_map();
-  map<string, string>::iterator iter;
-  for (iter = m.begin(); iter != m.end(); ++iter) {
-    headers.push_back(pair<string, string>(iter->first, iter->second));
+  for (const auto& kv: new_env.get_map()) {
+    headers.emplace_back(kv);
   }
 
   map<string, string>& meta_map = new_info.x_meta_map;
-  for (iter = meta_map.begin(); iter != meta_map.end(); ++iter) {
-    headers.push_back(pair<string, string>(iter->first, iter->second));
+  for (const auto& kv: meta_map) {
+    headers.emplace_back(kv);
   }
 
   string params_str;
@@ -416,14 +411,14 @@ static void grants_by_type_add_perm(map<int, string>& grants_by_type, int perm,
   }
 }
 
-static void add_grants_headers(map<int, string>& grants, map<string, string, ltstr_nocase>& attrs, map<string, string>& meta_map)
+static void add_grants_headers(map<int, string>& grants, RGWEnv& env, map<string, string>& meta_map)
 {
   struct grant_type_to_header *t;
 
   for (t = grants_headers_def; t->header; t++) {
     map<int, string>::iterator iter = grants.find(t->type);
     if (iter != grants.end()) {
-      attrs[t->header] = iter->second;
+      env.set(t->header,iter->second);
       meta_map[t->header] = iter->second;
     }
   }
@@ -456,18 +451,15 @@ int RGWRESTStreamWriteRequest::put_obj_init(RGWAccessKey& key, rgw_obj& obj, uin
   new_info.script_uri.append(resource);
   new_info.request_uri = new_info.script_uri;
 
-  map<string, string, ltstr_nocase>& m = new_env.get_map();
-  map<string, bufferlist>::iterator bliter;
-
   /* merge send headers */
-  for (bliter = attrs.begin(); bliter != attrs.end(); ++bliter) {
-    bufferlist& bl = bliter->second;
-    const string& name = bliter->first;
+  for (auto& attr: attrs) {
+    bufferlist& bl = attr.second;
+    const string& name = attr.first;
     string val = bl.c_str();
     if (name.compare(0, sizeof(RGW_ATTR_META_PREFIX) - 1, RGW_ATTR_META_PREFIX) == 0) {
       string header_name = RGW_AMZ_META_PREFIX;
       header_name.append(name.substr(sizeof(RGW_ATTR_META_PREFIX) - 1));
-      m[header_name] = val;
+      new_env.set(header_name, val);
       new_info.x_meta_map[header_name] = val;
     }
   }
@@ -488,16 +480,15 @@ int RGWRESTStreamWriteRequest::put_obj_init(RGWAccessKey& key, rgw_obj& obj, uin
     ACLPermission& perm = grant.get_permission();
     grants_by_type_add_perm(grants_by_type, perm.get_permissions(), grant);
   }
-  add_grants_headers(grants_by_type, m, new_info.x_meta_map);
+  add_grants_headers(grants_by_type, new_env, new_info.x_meta_map);
   ret = sign_request(key, new_env, new_info);
   if (ret < 0) {
     ldout(cct, 0) << "ERROR: failed to sign request" << dendl;
     return ret;
   }
 
-  map<string, string>::iterator iter;
-  for (iter = m.begin(); iter != m.end(); ++iter) {
-    headers.push_back(pair<string, string>(iter->first, iter->second));
+  for (const auto& kv: new_env.get_map()) {
+    headers.emplace_back(kv);
   }
 
   cb = new RGWRESTStreamOutCB(this);
@@ -682,10 +673,8 @@ int RGWRESTStreamRWRequest::send_request(RGWAccessKey *key, map<string, string>&
     }
   }
 
-  map<string, string, ltstr_nocase>& m = new_env.get_map();
-  map<string, string>::iterator iter;
-  for (iter = m.begin(); iter != m.end(); ++iter) {
-    headers.push_back(pair<string, string>(iter->first, iter->second));
+  for (const auto& kv: new_env.get_map()) {
+    headers.emplace_back(kv);
   }
 
   bool send_data_hint = false;

diff --git a/src/rgw/rgw_rest_conn.cc b/src/rgw/rgw_rest_conn.cc
index 6496f129d792ffecbe0069cbf02c80c6628ce89b..efac447282d9129efbb554e1753141b8e803cef6 100644 (file)
--- a/src/rgw/rgw_rest_conn.cc
+++ b/src/rgw/rgw_rest_conn.cc
@@ -180,15 +180,15 @@ int RGWRESTConn::get_obj(const rgw_user& uid, req_info *info /* optional */, rgw
   }
   map<string, string> extra_headers;
   if (info) {
-    map<string, string, ltstr_nocase>& orig_map = info->env->get_map();
+    const auto& orig_map = info->env->get_map();
 
     /* add original headers that start with HTTP_X_AMZ_ */
-#define SEARCH_AMZ_PREFIX "HTTP_X_AMZ_"
-    for (map<string, string>::iterator iter = orig_map.lower_bound(SEARCH_AMZ_PREFIX); iter != orig_map.end(); ++iter) {
+    static constexpr char SEARCH_AMZ_PREFIX[] = "HTTP_X_AMZ_";
+    for (auto iter= orig_map.lower_bound(SEARCH_AMZ_PREFIX); iter != orig_map.end(); ++iter) {
       const string& name = iter->first;
       if (name == "HTTP_X_AMZ_DATE") /* dont forward date from original request */
         continue;
-      if (name.compare(0, sizeof(SEARCH_AMZ_PREFIX) - 1, "HTTP_X_AMZ_") != 0)
+      if (name.compare(0, strlen(SEARCH_AMZ_PREFIX), SEARCH_AMZ_PREFIX) != 0)
         break;
       extra_headers[iter->first] = iter->second;
     }

diff --git a/src/rgw/rgw_rest_swift.cc b/src/rgw/rgw_rest_swift.cc
index 48708ab7f00784b545f63362b67b1e658bb9c974..a3ccdca781a634bb58c2fa1d7ed52ec1e9db3636 100644 (file)
--- a/src/rgw/rgw_rest_swift.cc
+++ b/src/rgw/rgw_rest_swift.cc
@@ -571,20 +571,18 @@ static void get_rmattrs_from_headers(const req_state * const s,
                                     const char * const del_prefix,
                                     set<string>& rmattr_names)
 {
-  map<string, string, ltstr_nocase>& m = s->info.env->get_map();
-  map<string, string, ltstr_nocase>::const_iterator iter;
   const size_t put_prefix_len = strlen(put_prefix);
   const size_t del_prefix_len = strlen(del_prefix);
 
-  for (iter = m.begin(); iter != m.end(); ++iter) {
+  for (const auto& kv : s->info.env->get_map()) {
     size_t prefix_len = 0;
-    const char * const p = iter->first.c_str();
+    const char * const p = kv.first.c_str();
 
     if (strncasecmp(p, del_prefix, del_prefix_len) == 0) {
       /* Explicitly requested removal. */
       prefix_len = del_prefix_len;
     } else if ((strncasecmp(p, put_prefix, put_prefix_len) == 0)
-              && iter->second.empty()) {
+              && kv.second.empty()) {
       /* Removal requested by putting an empty value. */
       prefix_len = put_prefix_len;
     }