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

osd: replace object_info_t::operator=() with decode() #13938

Merged
merged 1 commit into from Mar 20, 2017
Merged

osd: replace object_info_t::operator=() with decode() #13938

merged 1 commit into from Mar 20, 2017

Conversation

jimmyway
Copy link

Signed-off-by: tang.jin tang.jin@istuary.com

@jimmyway
Copy link
Author

@dang

@jimmyway
Copy link
Author

@danderson

@@ -4117,7 +4117,7 @@ struct object_info_t {
explicit object_info_t(bufferlist& bl) {
decode(bl);
}
object_info_t operator=(bufferlist& bl) {
object_info_t &operator=(bufferlist& bl) {
Copy link
Contributor

Choose a reason for hiding this comment

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

i think we should remove this method instead. and use decode() when appropriate.

@dzafman what do you think?

Copy link
Author

Choose a reason for hiding this comment

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

@tchaikov May I delete this function directly or append '= delete' to the function declaration?

Copy link
Contributor

@tchaikov tchaikov Mar 16, 2017

Choose a reason for hiding this comment

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

as long as there is no users of operator=, i don't see why we cannot delete the default copy assignment operator.

Copy link
Author

Choose a reason for hiding this comment

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

OK, I will delete it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jimmyway please try compile the tree first. PG::_scan_snaps() uses this function, that's why i suggested use decode() directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

and what is deleted in your change is not the default copy assignment operator. the one created by compiler by default is

object_info_t& operator=(object_info_t&)

Copy link
Author

Choose a reason for hiding this comment

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

sorry, I made mistakes, I will check it at first.

Copy link
Author

Choose a reason for hiding this comment

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

Do you mean delete 'operation=' member function, and replace it to decode() in PG::_scan_snaps() ?

@jimmyway jimmyway changed the title osd: change return value from object to refs in order to eliminate extra object copy osd: delete the default copy assignment operator Mar 16, 2017
@jimmyway jimmyway changed the title osd: delete the default copy assignment operator osd: delete the unused copy assignment operator, replace it to decode() Mar 16, 2017
@jimmyway jimmyway changed the title osd: delete the unused copy assignment operator, replace it to decode() osd: delete the copy assignment operator, replace it to decode() Mar 16, 2017
Signed-off-by: tang.jin <tang.jin@istuary.com>
@jimmyway
Copy link
Author

@tchaikov

@tchaikov tchaikov changed the title osd: delete the copy assignment operator, replace it to decode() osd: replace object_info_t::operator=() with decode() Mar 17, 2017
@yuriw yuriw merged commit 80a10ec into ceph:master Mar 20, 2017
oritwas pushed a commit to oritwas/ceph that referenced this pull request Apr 21, 2017
Fix ceph#13938
Signed-off-by: Haomai Wang <haomai@xsky.com>

(cherry picked from commit dc6d6ce)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants