Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

disable copy constructor and copy assignment #980

Merged
merged 9 commits into from
Nov 12, 2020
Merged

Conversation

allenhan2
Copy link
Contributor

@allenhan2 allenhan2 commented Oct 16, 2020

Due to the way those types are initialized, it is not safe to allow the copy constructor or copy-assignment operator on the kv table type.

Change Description

  • disable copy constructor and assignment
  • disable move constructor and assignment

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

Doc team, please make sure our examples were not using these accidentally, thanks.

libraries/eosiolib/contracts/eosio/key_value.hpp Outdated Show resolved Hide resolved
libraries/eosiolib/contracts/eosio/key_value.hpp Outdated Show resolved Hide resolved
Copy link
Contributor

@larryk85 larryk85 left a comment

Choose a reason for hiding this comment

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

What is currently unsafe about it?

@tbfleming
Copy link
Contributor

It holds internal pointers to index members

@tbfleming
Copy link
Contributor

Which, BTW, is also incompatible with move. That should also be disabled.

@allenhan2
Copy link
Contributor Author

Which, BTW, is also incompatible with move. That should also be disabled.

Could you please elaborate a scenario for problem of move? thanks!

@tbfleming
Copy link
Contributor

Simplification of what's going on:

struct foo {
    int a = 0;
    int* p;

    foo(): p{&a} {}
};

@allenhan2
Copy link
Contributor Author

It is a great example, but could it happen in the table class? disable move will make hard for utility functions.

@tbfleming
Copy link
Contributor

Like I said above, kv_table does this. It's part of how it keeps track of indexes.

@allenhan2
Copy link
Contributor Author

okay, then, will disable the move.

@allenhan2 allenhan2 requested a review from b1bart November 9, 2020 16:45
@allenhan2
Copy link
Contributor Author

no permission to merge, submitted tick for permission, https://blockone.freshservice.com/support/tickets/15178

@allenhan2
Copy link
Contributor Author

@b1bart could you give another review, the merge is blocked by the request change, thanks,

@larryk85 larryk85 merged commit 329780f into develop Nov 12, 2020
@larryk85 larryk85 deleted the disable_copy branch November 12, 2020 20:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants