Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RGW: change function parameters from value to refrence #18355

Merged
merged 1 commit into from Oct 18, 2017

Conversation

gaosibei
Copy link

We use refrence instead of value
Signed-off-by: Sibei Gao gaosb@inspur.com

@gaosibei gaosibei force-pushed the wip-rgw-refrence branch 2 times, most recently from c90633f to 58d9c76 Compare October 17, 2017 14:29
@@ -86,7 +86,7 @@ class RGWAccessControlPolicy_S3 : public RGWAccessControlPolicy, public XMLObj
int rebuild(RGWRados *store, ACLOwner *owner, RGWAccessControlPolicy& dest);
bool compare_group_name(string& id, ACLGroupTypeEnum group) override;

virtual int create_canned(ACLOwner& _owner, ACLOwner& bucket_owner, string canned_acl) {
virtual int create_canned(ACLOwner& _owner, ACLOwner& bucket_owner, string& canned_acl) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const string&

@@ -455,7 +455,7 @@ class LocalApplier : public IdentityApplier {

LocalApplier(CephContext* const cct,
const RGWUserInfo& user_info,
std::string subuser)
const std::string& subuser)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid that taking subuser by const & here isn't the best option. It would make moving internal data of the incoming std::string to the subuser member impossible. Please consider a scenario when someone instantiate LocalApplier with a temporal variable as the subuser. It can perfectly bind const & but std::move will result in const && in such case, I guess.

Signed-off-by: Sibei Gao  <gaosb@inspur.com>
@gaosibei
Copy link
Author

@rzarzynski @cbodley
Thanks for your suggestion.
According to your review, I have modified the code.

Copy link
Contributor

@rzarzynski rzarzynski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks, @gaosibei!

@cbodley cbodley merged commit 50ba674 into ceph:master Oct 18, 2017
@gaosibei gaosibei deleted the wip-rgw-refrence branch October 24, 2017 00:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants