From 5791e2222877ad4f2e84c79cc7aa867ee1361976 Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Sat, 18 Nov 2023 11:43:58 -0500 Subject: [PATCH] rgw: reorder rgw_user members for default operator<=> the default operator<=> does a memberwise comparison the same that rgw_user::compare() did, except that it compared `ns` before `id` reorder the rgw_user members so that the default operator<=> can replace compare() and the related comparison operators replaces uses of rgw_user::compare() with operator== and != Signed-off-by: Casey Bodley --- src/rgw/driver/rados/rgw_user.cc | 16 ++++++------ src/rgw/rgw_acl_s3.cc | 6 ++--- src/rgw/rgw_op.cc | 2 +- src/rgw/rgw_user_types.h | 45 +++++--------------------------- src/test/test_rgw_admin_meta.cc | 2 +- 5 files changed, 20 insertions(+), 51 deletions(-) diff --git a/src/rgw/driver/rados/rgw_user.cc b/src/rgw/driver/rados/rgw_user.cc index 506ac0acf11..b5569e481c5 100644 --- a/src/rgw/driver/rados/rgw_user.cc +++ b/src/rgw/driver/rados/rgw_user.cc @@ -237,7 +237,7 @@ int RGWAccessKeyPool::init(RGWUserAdminOpState& op_state) } const rgw_user& uid = op_state.get_user_id(); - if (uid.compare(RGW_USER_ANON_ID) == 0) { + if (uid == rgw_user(RGW_USER_ANON_ID)) { keys_allowed = false; return -EINVAL; } @@ -890,7 +890,7 @@ int RGWSubUserPool::init(RGWUserAdminOpState& op_state) } const rgw_user& uid = op_state.get_user_id(); - if (uid.compare(RGW_USER_ANON_ID) == 0) { + if (uid == rgw_user(RGW_USER_ANON_ID)) { subusers_allowed = false; return -EACCES; } @@ -1198,7 +1198,7 @@ int RGWUserCapPool::init(RGWUserAdminOpState& op_state) } const rgw_user& uid = op_state.get_user_id(); - if (uid.compare(RGW_USER_ANON_ID) == 0) { + if (uid == rgw_user(RGW_USER_ANON_ID)) { caps_allowed = false; return -EACCES; } @@ -1373,7 +1373,7 @@ int RGWUser::init(const DoutPrefixProvider *dpp, RGWUserAdminOpState& op_state, } } - if (!user_id.empty() && (user_id.compare(RGW_USER_ANON_ID) != 0)) { + if (!user_id.empty() && user_id != rgw_user(RGW_USER_ANON_ID)) { user = driver->get_user(user_id); found = (user->load_user(dpp, y) >= 0); op_state.found_by_uid = found; @@ -1477,12 +1477,12 @@ int RGWUser::check_op(RGWUserAdminOpState& op_state, std::string *err_msg) int ret = 0; const rgw_user& uid = op_state.get_user_id(); - if (uid.compare(RGW_USER_ANON_ID) == 0) { + if (uid == rgw_user(RGW_USER_ANON_ID)) { set_err_msg(err_msg, "unable to perform operations on the anonymous user"); return -EINVAL; } - if (is_populated() && user_id.compare(uid) != 0) { + if (is_populated() && user_id != uid) { set_err_msg(err_msg, "user id mismatch, operation id: " + uid.to_str() + " does not match: " + user_id.to_str()); @@ -1858,7 +1858,7 @@ int RGWUser::execute_modify(const DoutPrefixProvider *dpp, RGWUserAdminOpState& } // ensure that we can modify the user's attributes - if (user_id.compare(RGW_USER_ANON_ID) == 0) { + if (user_id == rgw_user(RGW_USER_ANON_ID)) { set_err_msg(err_msg, "unable to modify anonymous user's info"); return -EACCES; } @@ -1870,7 +1870,7 @@ int RGWUser::execute_modify(const DoutPrefixProvider *dpp, RGWUserAdminOpState& // make sure we are not adding a duplicate email if (old_email != op_email) { ret = driver->get_user_by_email(dpp, op_email, y, &duplicate_check); - if (ret >= 0 && duplicate_check->get_id().compare(user_id) != 0) { + if (ret >= 0 && duplicate_check->get_id() != user_id) { set_err_msg(err_msg, "cannot add duplicate email"); return -ERR_EMAIL_EXIST; } diff --git a/src/rgw/rgw_acl_s3.cc b/src/rgw/rgw_acl_s3.cc index 9f71e328150..3fc072a33ca 100644 --- a/src/rgw/rgw_acl_s3.cc +++ b/src/rgw/rgw_acl_s3.cc @@ -392,11 +392,11 @@ int RGWAccessControlList_S3::create_canned(ACLOwner& owner, ACLOwner& bucket_own 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.compare(owner.get_id()) != 0) + if (bid != owner.get_id()) 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.compare(owner.get_id()) != 0) + if (bid != owner.get_id()) add_grant(&bucket_owner_grant); } else { return -EINVAL; @@ -489,7 +489,7 @@ int RGWAccessControlPolicy_S3::rebuild(const DoutPrefixProvider *dpp, ACLOwner *requested_owner = static_cast(find_first("Owner")); if (requested_owner) { rgw_user& requested_id = requested_owner->get_id(); - if (!requested_id.empty() && requested_id.compare(owner->get_id()) != 0) + if (!requested_id.empty() && requested_id != owner->get_id()) return -EPERM; } diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index a290388efe0..2193ff104c9 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -457,7 +457,7 @@ static int read_obj_policy(const DoutPrefixProvider *dpp, return ret; } const rgw_user& bucket_owner = bucket_policy.get_owner().get_id(); - if (bucket_owner.compare(s->user->get_id()) != 0 && + if (bucket_owner != s->user->get_id() && ! s->auth.identity->is_admin_of(bucket_owner)) { auto r = eval_identity_or_session_policies(dpp, s->iam_user_policies, s->env, rgw::IAM::s3ListBucket, ARN(bucket->get_key())); diff --git a/src/rgw/rgw_user_types.h b/src/rgw/rgw_user_types.h index c9a1a46ade1..1aaf4cfa5d3 100644 --- a/src/rgw/rgw_user_types.h +++ b/src/rgw/rgw_user_types.h @@ -26,9 +26,10 @@ #include "common/Formatter.h" struct rgw_user { + // note: order of member variables matches the sort order of operator<=> std::string tenant; - std::string id; std::string ns; + std::string id; rgw_user() {} explicit rgw_user(const std::string& s) { @@ -36,13 +37,13 @@ struct rgw_user { } rgw_user(const std::string& tenant, const std::string& id, const std::string& ns="") : tenant(tenant), - id(id), - ns(ns) { + ns(ns), + id(id) { } rgw_user(std::string&& tenant, std::string&& id, std::string&& ns="") : tenant(std::move(tenant)), - id(std::move(id)), - ns(std::move(ns)) { + ns(std::move(ns)), + id(std::move(id)) { } void encode(ceph::buffer::list& bl) const { @@ -118,40 +119,8 @@ struct rgw_user { return *this; } - int compare(const rgw_user& u) const { - int r = tenant.compare(u.tenant); - if (r != 0) - return r; - r = ns.compare(u.ns); - if (r != 0) { - return r; - } - return id.compare(u.id); - } - int compare(const std::string& str) const { - rgw_user u(str); - return compare(u); - } + friend auto operator<=>(const rgw_user&, const rgw_user&) = default; - bool operator!=(const rgw_user& rhs) const { - return (compare(rhs) != 0); - } - bool operator==(const rgw_user& rhs) const { - return (compare(rhs) == 0); - } - bool operator<(const rgw_user& rhs) const { - if (tenant < rhs.tenant) { - return true; - } else if (tenant > rhs.tenant) { - return false; - } - if (ns < rhs.ns) { - return true; - } else if (ns > rhs.ns) { - return false; - } - return (id < rhs.id); - } void dump(ceph::Formatter *f) const; static void generate_test_instances(std::list& o); }; diff --git a/src/test/test_rgw_admin_meta.cc b/src/test/test_rgw_admin_meta.cc index b1d5fad0600..00c43d10b54 100644 --- a/src/test/test_rgw_admin_meta.cc +++ b/src/test/test_rgw_admin_meta.cc @@ -460,7 +460,7 @@ int compare_access_keys(RGWAccessKey& k1, RGWAccessKey& k2) { int compare_user_info(RGWUserInfo& i1, RGWUserInfo& i2) { int rv; - if ((rv = i1.user_id.compare(i2.user_id)) != 0) + if ((rv = i1.user_id.id.compare(i2.user_id.id)) != 0) return rv; if ((rv = i1.display_name.compare(i2.display_name)) != 0) return rv; -- 2.39.5