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

mds: use find in is_data_pool #15982

Merged
merged 1 commit into from Jul 18, 2017
Merged

Conversation

jan--f
Copy link
Contributor

@jan--f jan--f commented Jun 28, 2017

binary_search assumes that vector data_pools is ordered. add_data_pool
however only appends to the vector, so no order can be assumed.

Fixes: http://tracker.ceph.com/issues/20452

Signed-off-by: Jan Fajerski jfajerski@suse.com

@batrick batrick added bug-fix cephfs Ceph File System labels Jun 28, 2017
@batrick
Copy link
Member

batrick commented Jun 28, 2017

Reviewed-by: Patrick Donnelly <pdonnell@redhat.com>

src/mds/MDSMap.h Outdated
return std::binary_search(data_pools.begin(), data_pools.end(), poolid);
std::vector<int64_t>::const_iterator b = data_pools.begin();
std::vector<int64_t>::const_iterator e = data_pools.end();
std::vector<int64_t>::const_iterator p = std::find(b, e, poolid);
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch!

Above three lines can be simplified into
"auto p = std::find(data_pools.begin(), data_pools.end(), poolid);"

Copy link
Contributor Author

@jan--f jan--f Jun 29, 2017

Choose a reason for hiding this comment

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

Thx, one more question:
Should we return p != data_pools::end() or is the more explicit return better?

Copy link
Member

Choose a reason for hiding this comment

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

Either is fine with me.

binary_search assumes that vector data_pools is ordered. add_data_pool
however only appends to the vector, so no order can be assumed.

Fixes: http://tracker.ceph.com/issues/20452

Signed-off-by: Jan Fajerski <jfajerski@suse.com>
@batrick batrick merged commit fe6d71c into ceph:master Jul 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants