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/rgw_swift_auth.cc: using string::back() instead as the C++11 recommend #14827

Merged
merged 1 commit into from May 19, 2017

Conversation

Projects
None yet
6 participants
@liuyuhong66
Contributor

liuyuhong66 commented Apr 27, 2017

Signed-off-by: liuyuhong liuyuhong@cmss.chinamobile.com

@liuyuhong66

This comment has been minimized.

Contributor

liuyuhong66 commented Apr 27, 2017

@Jing-Scott @rzarzynski please help me to have a look, tks

@rzarzynski rzarzynski self-assigned this Apr 27, 2017

@Jing-Scott

This comment has been minimized.

Contributor

Jing-Scott commented Apr 27, 2017

looks good!

@rzarzynski

@liuyuhong66: LGTM, thanks!

@@ -304,7 +304,7 @@ ExternalTokenEngine::authenticate(const std::string& token,
}
std::string auth_url = g_conf->rgw_swift_auth_url;
if (auth_url[auth_url.length() - 1] != '/') {
if (auth_url.back() != '/') {

This comment has been minimized.

@rzarzynski

rzarzynski Apr 27, 2017

Contributor

May we have a comment saying thatExternalTokenEngine::is_applicable guarantees rgw_swift_auth_url is non-empty?

Anyway, it's only a suggestion. Even now the commit is perfectly mergeable.

This comment has been minimized.

@yehudasa

yehudasa Apr 27, 2017

Member

@liuyuhong66 just modify it to if (!auth_url.empty() && auth_url.back() != '/')

@liuyuhong66

This comment has been minimized.

Contributor

liuyuhong66 commented Apr 28, 2017

@rzarzynski @yehudasa i did some changes,tks

@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Apr 28, 2017

LGTM! It will be merged soon.

@@ -304,7 +304,7 @@ ExternalTokenEngine::authenticate(const std::string& token,
}
std::string auth_url = g_conf->rgw_swift_auth_url;
if (auth_url[auth_url.length() - 1] != '/') {
if (!auth_url.empty() && auth_url.back() != '/') {

This comment has been minimized.

@tchaikov

tchaikov Apr 29, 2017

Contributor

i don't think the check for empty is necessary. is_applicable() will return right away in this case.

@tchaikov

better remove !auth_url.empty()

rgw/rgw_swift_auth.cc: using string::back() instead as the C++11 reco…
…mmend

Signed-off-by: liuyuhong <liuyuhong@cmss.chinamobile.com>
@liuyuhong66

This comment has been minimized.

Contributor

liuyuhong66 commented May 2, 2017

@tchaikov ,tks, just like you said !auth_url.empty() is unnecessary here

@tchaikov tchaikov requested a review from yehudasa May 2, 2017

@cbodley

This comment has been minimized.

@cbodley cbodley merged commit aa16248 into ceph:master May 19, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment