From fb7c5b45a7a95636da36404f3dcffdd8e7cbb0f4 Mon Sep 17 00:00:00 2001 From: Yuval Lifshitz Date: Sun, 4 Oct 2020 16:04:45 +0300 Subject: [PATCH] rgw/file: add missing brackets around if statement also some small style issues issues were detected by pvs-studio static analyzer Signed-off-by: Yuval Lifshitz --- src/rgw/rgw_file.cc | 76 ++++++------ src/rgw/rgw_file.h | 282 ++++++++++++++++++++++---------------------- 2 files changed, 183 insertions(+), 175 deletions(-) diff --git a/src/rgw/rgw_file.cc b/src/rgw/rgw_file.cc index 442096d1e2d..52fdbd93504 100644 --- a/src/rgw/rgw_file.cc +++ b/src/rgw/rgw_file.cc @@ -572,6 +572,7 @@ namespace rgw { if (! rc) { /* conflict! */ rc = rgw_fh_rele(get_fs(), lfh, RGW_FH_RELE_FLAG_NONE); + // ignore return code return MkObjResult{nullptr, -EEXIST}; } @@ -688,6 +689,7 @@ namespace rgw { if (! rc) { /* conflict! */ rc = rgw_fh_rele(get_fs(), lfh, RGW_FH_RELE_FLAG_NONE); + // ignore return code return MkObjResult{nullptr, -EEXIST}; } @@ -758,6 +760,7 @@ namespace rgw { if (! rc) { /* conflict! */ rc = rgw_fh_rele(get_fs(), lfh, RGW_FH_RELE_FLAG_NONE); + // ignore return code return MkObjResult{nullptr, -EEXIST}; } @@ -973,7 +976,7 @@ namespace rgw { explicit ObjUnref(RGWLibFS* _fs) : fs(_fs) {} void operator()(RGWFileHandle* fh) const { lsubdout(fs->get_context(), rgw, 5) - << __func__ + << __PRETTY_FUNCTION__ << fh->name << " before ObjUnref refs=" << fh->get_refcnt() << dendl; @@ -1520,11 +1523,11 @@ namespace rgw { } int RGWWriteRequest::exec_start() { - struct req_state* s = get_state(); + struct req_state* state = get_state(); auto compression_type = get_store()->svc()->zone->get_zone_params().get_compression_type( - s->bucket->get_placement_rule()); + state->bucket->get_placement_rule()); /* not obviously supportable */ ceph_assert(! dlo_manifest); @@ -1533,8 +1536,8 @@ namespace rgw { perfcounter->inc(l_rgw_put); op_ret = -EINVAL; - if (s->object->empty()) { - ldout(s->cct, 0) << __func__ << " called on empty object" << dendl; + if (state->object->empty()) { + ldout(state->cct, 0) << __func__ << " called on empty object" << dendl; goto done; } @@ -1542,7 +1545,7 @@ namespace rgw { if (op_ret < 0) goto done; - op_ret = get_system_versioning_params(s, &olh_epoch, &version_id); + op_ret = get_system_versioning_params(state, &olh_epoch, &version_id); if (op_ret < 0) { goto done; } @@ -1552,36 +1555,36 @@ namespace rgw { /* skipping user-supplied etag--we might have one in future, but * like data it and other attrs would arrive after open */ - aio.emplace(s->cct->_conf->rgw_put_obj_min_window_size); + aio.emplace(state->cct->_conf->rgw_put_obj_min_window_size); - if (s->bucket->versioning_enabled()) { + if (state->bucket->versioning_enabled()) { if (!version_id.empty()) { - s->object->set_instance(version_id); + state->object->set_instance(version_id); } else { - s->object->gen_rand_obj_instance_name(); - version_id = s->object->get_instance(); + state->object->gen_rand_obj_instance_name(); + version_id = state->object->get_instance(); } } - processor.emplace(&*aio, get_store(), s->bucket.get(), - &s->dest_placement, - s->bucket_owner.get_id(), - *static_cast(s->obj_ctx), - s->object->get_obj(), olh_epoch, s->req_id, this, s->yield); + processor.emplace(&*aio, get_store(), state->bucket.get(), + &state->dest_placement, + state->bucket_owner.get_id(), + *static_cast(state->obj_ctx), + state->object->get_obj(), olh_epoch, state->req_id, this, state->yield); - op_ret = processor->prepare(s->yield); + op_ret = processor->prepare(state->yield); if (op_ret < 0) { - ldout(s->cct, 20) << "processor->prepare() returned ret=" << op_ret + ldout(state->cct, 20) << "processor->prepare() returned ret=" << op_ret << dendl; goto done; } filter = &*processor; if (compression_type != "none") { - plugin = Compressor::create(s->cct, compression_type); + plugin = Compressor::create(state->cct, compression_type); if (! plugin) { - ldout(s->cct, 1) << "Cannot load plugin for rgw_compression_type " + ldout(state->cct, 1) << "Cannot load plugin for rgw_compression_type " << compression_type << dendl; } else { - compressor.emplace(s->cct, plugin, filter); + compressor.emplace(state->cct, plugin, filter); filter = &*compressor; } } @@ -1592,19 +1595,19 @@ namespace rgw { int RGWWriteRequest::exec_continue() { - struct req_state* s = get_state(); + struct req_state* state = get_state(); op_ret = 0; /* check guards (e.g., contig write) */ if (eio) { - ldout(s->cct, 5) + ldout(state->cct, 5) << " chunks arrived in wrong order" << " (mounting with -o sync required)" << dendl; return -EIO; } - op_ret = s->bucket->check_quota(user_quota, bucket_quota, real_ofs, true); + op_ret = state->bucket->check_quota(user_quota, bucket_quota, real_ofs, true); /* max_size exceed */ if (op_ret < 0) return -EIO; @@ -1630,23 +1633,23 @@ namespace rgw { map::iterator iter; char calc_md5[CEPH_CRYPTO_MD5_DIGESTSIZE * 2 + 1]; unsigned char m[CEPH_CRYPTO_MD5_DIGESTSIZE]; - struct req_state* s = get_state(); + struct req_state* state = get_state(); size_t osize = rgw_fh->get_size(); struct timespec octime = rgw_fh->get_ctime(); struct timespec omtime = rgw_fh->get_mtime(); real_time appx_t = real_clock::now(); - s->obj_size = bytes_written; - perfcounter->inc(l_rgw_put_b, s->obj_size); + state->obj_size = bytes_written; + perfcounter->inc(l_rgw_put_b, state->obj_size); // flush data in filters - op_ret = filter->process({}, s->obj_size); + op_ret = filter->process({}, state->obj_size); if (op_ret < 0) { goto done; } - op_ret = s->bucket->check_quota(user_quota, bucket_quota, s->obj_size, true); + op_ret = state->bucket->check_quota(user_quota, bucket_quota, state->obj_size, true); /* max_size exceed */ if (op_ret < 0) { goto done; @@ -1658,11 +1661,11 @@ namespace rgw { bufferlist tmp; RGWCompressionInfo cs_info; cs_info.compression_type = plugin->get_type_name(); - cs_info.orig_size = s->obj_size; + cs_info.orig_size = state->obj_size; cs_info.blocks = std::move(compressor->get_compression_blocks()); encode(cs_info, tmp); attrs[RGW_ATTR_COMPRESSION] = tmp; - ldout(s->cct, 20) << "storing " << RGW_ATTR_COMPRESSION + ldout(state->cct, 20) << "storing " << RGW_ATTR_COMPRESSION << " with type=" << cs_info.compression_type << ", orig_size=" << cs_info.orig_size << ", blocks=" << cs_info.blocks.size() << dendl; @@ -1686,14 +1689,14 @@ namespace rgw { emplace_attr(RGW_ATTR_UNIX_KEY1, std::move(ux_key)); emplace_attr(RGW_ATTR_UNIX1, std::move(ux_attrs)); - for (iter = s->generic_attrs.begin(); iter != s->generic_attrs.end(); + for (iter = state->generic_attrs.begin(); iter != state->generic_attrs.end(); ++iter) { buffer::list& attrbl = attrs[iter->first]; const string& val = iter->second; attrbl.append(val.c_str(), val.size() + 1); } - op_ret = rgw_get_request_metadata(s->cct, s->info, attrs); + op_ret = rgw_get_request_metadata(state->cct, state->info, attrs); if (op_ret < 0) { goto done; } @@ -1709,10 +1712,10 @@ namespace rgw { emplace_attr(RGW_ATTR_SLO_UINDICATOR, std::move(slo_userindicator_bl)); } - op_ret = processor->complete(s->obj_size, etag, &mtime, real_time(), attrs, + op_ret = processor->complete(state->obj_size, etag, &mtime, real_time(), attrs, (delete_at ? *delete_at : real_time()), if_match, if_nomatch, nullptr, nullptr, nullptr, - s->yield); + state->yield); if (op_ret != 0) { /* revert attr updates */ rgw_fh->set_mtime(omtime); @@ -1721,7 +1724,7 @@ namespace rgw { } done: - perfcounter->tinc(l_rgw_put_lat, s->time_elapsed()); + perfcounter->tinc(l_rgw_put_lat, state->time_elapsed()); return op_ret; } /* exec_finish */ @@ -2383,6 +2386,7 @@ int rgw_writev(struct rgw_fs *rgw_fs, struct rgw_file_handle *fh, rgw_uio *uio, uint32_t flags) { + // not supported - rest of function is ignored return -ENOTSUP; CephContext* cct = static_cast(rgw_fs->rgw); diff --git a/src/rgw/rgw_file.h b/src/rgw/rgw_file.h index d4a4d43be5a..103e0f651d2 100644 --- a/src/rgw/rgw_file.h +++ b/src/rgw/rgw_file.h @@ -216,7 +216,7 @@ namespace rgw { struct timespec mtime; struct timespec atime; uint32_t version; - State() : dev(0), size(0), nlink(1), owner_uid(0), owner_gid(0), + State() : dev(0), size(0), nlink(1), owner_uid(0), owner_gid(0), unix_mode(0), ctime{0,0}, mtime{0,0}, atime{0,0}, version(0) {} } state; @@ -1355,16 +1355,16 @@ public: } int header_init() override { - struct req_state* s = get_state(); - s->info.method = "GET"; - s->op = OP_GET; + struct req_state* state = get_state(); + state->info.method = "GET"; + state->op = OP_GET; /* XXX derp derp derp */ - s->relative_uri = "/"; - s->info.request_uri = "/"; // XXX - s->info.effective_uri = "/"; - s->info.request_params = ""; - s->info.domain = ""; /* XXX ? */ + state->relative_uri = "/"; + state->info.request_uri = "/"; // XXX + state->info.effective_uri = "/"; + state->info.request_params = ""; + state->info.domain = ""; /* XXX ? */ return 0; } @@ -1491,17 +1491,17 @@ public: } int header_init() override { - struct req_state* s = get_state(); - s->info.method = "GET"; - s->op = OP_GET; + struct req_state* state = get_state(); + state->info.method = "GET"; + state->op = OP_GET; /* XXX derp derp derp */ std::string uri = "/" + rgw_fh->bucket_name() + "/"; - s->relative_uri = uri; - s->info.request_uri = uri; // XXX - s->info.effective_uri = uri; - s->info.request_params = ""; - s->info.domain = ""; /* XXX ? */ + state->relative_uri = uri; + state->info.request_uri = uri; // XXX + state->info.effective_uri = uri; + state->info.request_params = ""; + state->info.domain = ""; /* XXX ? */ prefix = rgw_fh->relative_object_name(); if (prefix.length() > 0) @@ -1551,7 +1551,7 @@ public: } void send_response() override { - struct req_state* s = get_state(); + struct req_state* state = get_state(); for (const auto& iter : objs) { std::string_view sref {iter.key.name}; @@ -1569,7 +1569,7 @@ public: lsubdout(cct, rgw, 15) << "RGWReaddirRequest " << __func__ << " " - << "list uri=" << s->relative_uri << " " + << "list uri=" << state->relative_uri << " " << " prefix=" << prefix << " " << " obj path=" << iter.key.name << " (" << sref << ")" << "" @@ -1617,7 +1617,7 @@ public: lsubdout(cct, rgw, 15) << "RGWReaddirRequest " << __func__ << " " - << "list uri=" << s->relative_uri << " " + << "list uri=" << state->relative_uri << " " << " prefix=" << prefix << " " << " cpref=" << sref << dendl; @@ -1696,16 +1696,16 @@ public: } int header_init() override { - struct req_state* s = get_state(); - s->info.method = "GET"; - s->op = OP_GET; + struct req_state* state = get_state(); + state->info.method = "GET"; + state->op = OP_GET; std::string uri = "/" + rgw_fh->bucket_name() + "/"; - s->relative_uri = uri; - s->info.request_uri = uri; - s->info.effective_uri = uri; - s->info.request_params = ""; - s->info.domain = ""; /* XXX ? */ + state->relative_uri = uri; + state->info.request_uri = uri; + state->info.effective_uri = uri; + state->info.request_params = ""; + state->info.domain = ""; /* XXX ? */ prefix = rgw_fh->relative_object_name(); if (prefix.length() > 0) @@ -1779,26 +1779,26 @@ public: int header_init() override { - struct req_state* s = get_state(); - s->info.method = "PUT"; - s->op = OP_PUT; + struct req_state* state = get_state(); + state->info.method = "PUT"; + state->op = OP_PUT; string uri = "/" + bucket_name; /* XXX derp derp derp */ - s->relative_uri = uri; - s->info.request_uri = uri; // XXX - s->info.effective_uri = uri; - s->info.request_params = ""; - s->info.domain = ""; /* XXX ? */ + state->relative_uri = uri; + state->info.request_uri = uri; // XXX + state->info.effective_uri = uri; + state->info.request_params = ""; + state->info.domain = ""; /* XXX ? */ return 0; } int get_params() override { - struct req_state* s = get_state(); - RGWAccessControlPolicy_S3 s3policy(s->cct); + struct req_state* state = get_state(); + RGWAccessControlPolicy_S3 s3policy(state->cct); /* we don't have (any) headers, so just create canned ACLs */ - int ret = s3policy.create_canned(s->owner, s->bucket_owner, s->canned_acl); + int ret = s3policy.create_canned(state->owner, state->bucket_owner, state->canned_acl); policy = s3policy; return ret; } @@ -1839,17 +1839,17 @@ public: int header_init() override { - struct req_state* s = get_state(); - s->info.method = "DELETE"; - s->op = OP_DELETE; + struct req_state* state = get_state(); + state->info.method = "DELETE"; + state->op = OP_DELETE; string uri = "/" + bucket_name; /* XXX derp derp derp */ - s->relative_uri = uri; - s->info.request_uri = uri; // XXX - s->info.effective_uri = uri; - s->info.request_params = ""; - s->info.domain = ""; /* XXX ? */ + state->relative_uri = uri; + state->info.request_uri = uri; // XXX + state->info.effective_uri = uri; + state->info.request_params = ""; + state->info.domain = ""; /* XXX ? */ return 0; } @@ -1898,29 +1898,29 @@ public: int header_init() override { - struct req_state* s = get_state(); - s->info.method = "PUT"; - s->op = OP_PUT; + struct req_state* state = get_state(); + state->info.method = "PUT"; + state->op = OP_PUT; /* XXX derp derp derp */ std::string uri = make_uri(bucket_name, obj_name); - s->relative_uri = uri; - s->info.request_uri = uri; // XXX - s->info.effective_uri = uri; - s->info.request_params = ""; - s->info.domain = ""; /* XXX ? */ + state->relative_uri = uri; + state->info.request_uri = uri; // XXX + state->info.effective_uri = uri; + state->info.request_params = ""; + state->info.domain = ""; /* XXX ? */ /* XXX required in RGWOp::execute() */ - s->content_length = bl.length(); + state->content_length = bl.length(); return 0; } int get_params() override { - struct req_state* s = get_state(); - RGWAccessControlPolicy_S3 s3policy(s->cct); + struct req_state* state = get_state(); + RGWAccessControlPolicy_S3 s3policy(state->cct); /* we don't have (any) headers, so just create canned ACLs */ - int ret = s3policy.create_canned(s->owner, s->bucket_owner, s->canned_acl); + int ret = s3policy.create_canned(state->owner, state->bucket_owner, state->canned_acl); policy = s3policy; return ret; } @@ -1992,17 +1992,17 @@ public: int header_init() override { - struct req_state* s = get_state(); - s->info.method = "GET"; - s->op = OP_GET; + struct req_state* state = get_state(); + state->info.method = "GET"; + state->op = OP_GET; /* XXX derp derp derp */ - s->relative_uri = make_uri(rgw_fh->bucket_name(), + state->relative_uri = make_uri(rgw_fh->bucket_name(), rgw_fh->relative_object_name()); - s->info.request_uri = s->relative_uri; // XXX - s->info.effective_uri = s->relative_uri; - s->info.request_params = ""; - s->info.domain = ""; /* XXX ? */ + state->info.request_uri = state->relative_uri; // XXX + state->info.effective_uri = state->relative_uri; + state->info.request_params = ""; + state->info.domain = ""; /* XXX ? */ return 0; } @@ -2074,17 +2074,17 @@ public: int header_init() override { - struct req_state* s = get_state(); - s->info.method = "DELETE"; - s->op = OP_DELETE; + struct req_state* state = get_state(); + state->info.method = "DELETE"; + state->op = OP_DELETE; /* XXX derp derp derp */ std::string uri = make_uri(bucket_name, obj_name); - s->relative_uri = uri; - s->info.request_uri = uri; // XXX - s->info.effective_uri = uri; - s->info.request_params = ""; - s->info.domain = ""; /* XXX ? */ + state->relative_uri = uri; + state->info.request_uri = uri; // XXX + state->info.effective_uri = uri; + state->info.request_params = ""; + state->info.domain = ""; /* XXX ? */ return 0; } @@ -2152,16 +2152,16 @@ public: int header_init() override { - struct req_state* s = get_state(); - s->info.method = "GET"; - s->op = OP_GET; + struct req_state* state = get_state(); + state->info.method = "GET"; + state->op = OP_GET; /* XXX derp derp derp */ - s->relative_uri = make_uri(bucket_name, obj_name); - s->info.request_uri = s->relative_uri; // XXX - s->info.effective_uri = s->relative_uri; - s->info.request_params = ""; - s->info.domain = ""; /* XXX ? */ + state->relative_uri = make_uri(bucket_name, obj_name); + state->info.request_uri = state->relative_uri; // XXX + state->info.effective_uri = state->relative_uri; + state->info.request_params = ""; + state->info.domain = ""; /* XXX ? */ return 0; } @@ -2229,16 +2229,16 @@ public: int header_init() override { - struct req_state* s = get_state(); - s->info.method = "GET"; - s->op = OP_GET; + struct req_state* state = get_state(); + state->info.method = "GET"; + state->op = OP_GET; /* XXX derp derp derp */ - s->relative_uri = uri; - s->info.request_uri = uri; // XXX - s->info.effective_uri = uri; - s->info.request_params = ""; - s->info.domain = ""; /* XXX ? */ + state->relative_uri = uri; + state->info.request_uri = uri; // XXX + state->info.effective_uri = uri; + state->info.request_params = ""; + state->info.domain = ""; /* XXX ? */ return 0; } @@ -2295,17 +2295,17 @@ public: int header_init() override { - struct req_state* s = get_state(); - s->info.method = "GET"; - s->op = OP_GET; + struct req_state* state = get_state(); + state->info.method = "GET"; + state->op = OP_GET; /* XXX derp derp derp */ std::string uri = "/" + rgw_fh->bucket_name() + "/"; - s->relative_uri = uri; - s->info.request_uri = uri; // XXX - s->info.effective_uri = uri; - s->info.request_params = ""; - s->info.domain = ""; /* XXX ? */ + state->relative_uri = uri; + state->info.request_uri = uri; // XXX + state->info.effective_uri = uri; + state->info.request_params = ""; + state->info.domain = ""; /* XXX ? */ prefix = rgw_fh->relative_object_name(); if (prefix.length() > 0) @@ -2322,39 +2322,41 @@ public: } void send_response() override { - struct req_state* s = get_state(); + struct req_state* state = get_state(); // try objects for (const auto& iter : objs) { auto& name = iter.key.name; lsubdout(cct, rgw, 15) << "RGWStatLeafRequest " << __func__ << " " - << "list uri=" << s->relative_uri << " " + << "list uri=" << state->relative_uri << " " << " prefix=" << prefix << " " << " obj path=" << name << "" << " target = " << path << "" << dendl; /* XXX is there a missing match-dir case (trailing '/')? */ matched = true; - if (name == path) + if (name == path) { exact_matched = true; - return; + return; + } } // try prefixes for (auto& iter : common_prefixes) { auto& name = iter.first; lsubdout(cct, rgw, 15) << "RGWStatLeafRequest " << __func__ << " " - << "list uri=" << s->relative_uri << " " + << "list uri=" << state->relative_uri << " " << " prefix=" << prefix << " " << " pref path=" << name << " (not chomped)" << " target = " << path << "" << dendl; matched = true; /* match-dir case (trailing '/') */ - if (name == prefix + "/") + if (name == prefix + "/") { exact_matched = true; - is_dir = true; - break; + is_dir = true; + return; + } } } @@ -2390,10 +2392,12 @@ public: RGWFileHandle* _fh, const std::string& _bname, const std::string& _oname) : RGWLibContinuedReq(store->ctx(), std::move(_user)), bucket_name(_bname), obj_name(_oname), - rgw_fh(_fh), filter(nullptr), real_ofs(0), + rgw_fh(_fh), filter(nullptr), timer_id(0), real_ofs(0), bytes_written(0), eio(false) { - int ret = header_init(); + // in ctr this is not a virtual call + // invoking this classes's header_init() + int ret = RGWWriteRequest::header_init(); if (ret == 0) { ret = init_from_header(store, get_state()); } @@ -2415,26 +2419,26 @@ public: int header_init() override { - struct req_state* s = get_state(); - s->info.method = "PUT"; - s->op = OP_PUT; + struct req_state* state = get_state(); + state->info.method = "PUT"; + state->op = OP_PUT; /* XXX derp derp derp */ std::string uri = make_uri(bucket_name, obj_name); - s->relative_uri = uri; - s->info.request_uri = uri; // XXX - s->info.effective_uri = uri; - s->info.request_params = ""; - s->info.domain = ""; /* XXX ? */ + state->relative_uri = uri; + state->info.request_uri = uri; // XXX + state->info.effective_uri = uri; + state->info.request_params = ""; + state->info.domain = ""; /* XXX ? */ return 0; } int get_params() override { - struct req_state* s = get_state(); - RGWAccessControlPolicy_S3 s3policy(s->cct); + struct req_state* state = get_state(); + RGWAccessControlPolicy_S3 s3policy(state->cct); /* we don't have (any) headers, so just create canned ACLs */ - int ret = s3policy.create_canned(s->owner, s->bucket_owner, s->canned_acl); + int ret = s3policy.create_canned(state->owner, state->bucket_owner, state->canned_acl); policy = s3policy; return ret; } @@ -2507,9 +2511,9 @@ public: int header_init() override { - struct req_state* s = get_state(); - s->info.method = "PUT"; // XXX check - s->op = OP_PUT; + struct req_state* state = get_state(); + state->info.method = "PUT"; // XXX check + state->op = OP_PUT; src_bucket_name = src_parent->bucket_name(); // need s->src_bucket_name? @@ -2533,11 +2537,11 @@ public: emplace_attr(RGW_ATTR_UNIX_KEY1, std::move(ux_key)); #if 0 /* XXX needed? */ - s->relative_uri = uri; - s->info.request_uri = uri; // XXX - s->info.effective_uri = uri; - s->info.request_params = ""; - s->info.domain = ""; /* XXX ? */ + state->relative_uri = uri; + state->info.request_uri = uri; // XXX + state->info.effective_uri = uri; + state->info.request_params = ""; + state->info.domain = ""; /* XXX ? */ #endif return 0; @@ -2585,17 +2589,17 @@ public: int header_init() override { - struct req_state* s = get_state(); - s->info.method = "PUT"; - s->op = OP_PUT; + struct req_state* state = get_state(); + state->info.method = "PUT"; + state->op = OP_PUT; /* XXX derp derp derp */ std::string uri = make_uri(bucket_name, obj_name); - s->relative_uri = uri; - s->info.request_uri = uri; // XXX - s->info.effective_uri = uri; - s->info.request_params = ""; - s->info.domain = ""; /* XXX ? */ + state->relative_uri = uri; + state->info.request_uri = uri; // XXX + state->info.effective_uri = uri; + state->info.request_params = ""; + state->info.domain = ""; /* XXX ? */ return 0; } @@ -2633,9 +2637,9 @@ public: } int header_init() override { - struct req_state* s = get_state(); - s->info.method = "GET"; - s->op = OP_GET; + struct req_state* state = get_state(); + state->info.method = "GET"; + state->op = OP_GET; return 0; } -- 2.39.5