From 1afcc3866ccaeaf35e154cda2df748d1799a9c92 Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Wed, 14 Jun 2017 14:27:05 +0200 Subject: [PATCH] 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 --- src/rgw/rgw_acl_s3.cc | 6 ++--- src/rgw/rgw_acl_s3.h | 2 +- src/rgw/rgw_common.cc | 24 ++++++++----------- src/rgw/rgw_common.h | 16 ++++++------- src/rgw/rgw_crypt.cc | 2 +- src/rgw/rgw_env.cc | 29 ++++++++++++++--------- src/rgw/rgw_rest_client.cc | 47 +++++++++++++++----------------------- src/rgw/rgw_rest_conn.cc | 8 +++---- src/rgw/rgw_rest_swift.cc | 8 +++---- 9 files changed, 66 insertions(+), 76 deletions(-) diff --git a/src/rgw/rgw_acl_s3.cc b/src/rgw/rgw_acl_s3.cc index 0eb3a78a0e2..eec7db6898a 100644 --- 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& _grants) { std::list 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 grants; diff --git a/src/rgw/rgw_acl_s3.h b/src/rgw/rgw_acl_s3.h index 6991a0b8363..149db40e70b 100644 --- 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 7b320b80488..8694805d59c 100644 --- 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& m = env->get_map(); - map::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::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 af3d7c15bb7..3b5c115fb60 100644 --- 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& get_map() { return env_map; } + const std::map& 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 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 9fb474c46f0..d2f5a443657 100644 --- 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* parts, diff --git a/src/rgw/rgw_env.cc b/src/rgw/rgw_env.cc index d1f876dcbee..bc195f71e76 100644 --- a/src/rgw/rgw_env.cc +++ b/src/rgw/rgw_env.cc @@ -50,7 +50,7 @@ const char *rgw_conf_get(const map& 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& 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::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::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::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 aa3de5fac83..f1ec7d72dae 100644 --- 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& m = env.get_map(); - if (cct->_conf->subsys.should_gather(ceph_subsys_rgw, 20)) { - map::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& m = new_env.get_map(); - map::iterator iter; - for (iter = m.begin(); iter != m.end(); ++iter) { - headers.push_back(pair(iter->first, iter->second)); + for (const auto& kv: new_env.get_map()) { + headers.emplace_back(kv); } map& meta_map = new_info.x_meta_map; - for (iter = meta_map.begin(); iter != meta_map.end(); ++iter) { - headers.push_back(pair(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& grants_by_type, int perm, } } -static void add_grants_headers(map& grants, map& attrs, map& meta_map) +static void add_grants_headers(map& grants, RGWEnv& env, map& meta_map) { struct grant_type_to_header *t; for (t = grants_headers_def; t->header; t++) { map::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& m = new_env.get_map(); - map::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::iterator iter; - for (iter = m.begin(); iter != m.end(); ++iter) { - headers.push_back(pair(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& } } - map& m = new_env.get_map(); - map::iterator iter; - for (iter = m.begin(); iter != m.end(); ++iter) { - headers.push_back(pair(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 6496f129d79..efac447282d 100644 --- 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 extra_headers; if (info) { - map& 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::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 48708ab7f00..a3ccdca781a 100644 --- 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& rmattr_names) { - map& m = s->info.env->get_map(); - map::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; } -- 2.39.5