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: remove duplicated calls to getattr #10858

Merged
merged 1 commit into from Aug 30, 2016

Conversation

atheism
Copy link
Contributor

@atheism atheism commented Aug 25, 2016

We do not need the duplicated calls to getattr. Remove the unnecessary code.

Signed-off-by: Weibing Zhang zhangweibing@unitedstack.com

@@ -1290,7 +1290,7 @@ int rgw_getattr(struct rgw_fs *rgw_fs,
rc = -(fs->getattr(rgw_fh, st));
}

return -(fs->getattr(rgw_fh, st));
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't remember what I was playing with here, but I think we want just this line (return -(expr)), and to zap the two calls that assign rc. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it would be OK. I will change the pull-request.

@mattbenjamin
Copy link
Contributor

@atheism you're right, this was some kind of experiement, the calls are idempotent, at least in the current code. I think maybe we should take the more concise form though.

@mattbenjamin
Copy link
Contributor

@atheism I'd tweak the commit msg--the reason to fs->getattr() (once :) is that it's the implementation of the API method.

Signed-off-by: Weibing Zhang <zhangweibing@unitedstack.com>
@atheism atheism changed the title rgw/rgw_file.cc: just return rc if we have already run fs->getattr() twice rgw: remove duplicated calls to getattr Aug 26, 2016
@cbodley
Copy link
Contributor

cbodley commented Aug 29, 2016

jenkins, retest this please

@mattbenjamin mattbenjamin merged commit de4ddd9 into ceph:master Aug 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants