[rpc] AsyncRPCOperation: Constructor takes std::shared_ptr<CWallet>#32
[rpc] AsyncRPCOperation: Constructor takes std::shared_ptr<CWallet>#32ch4ot1c wants to merge 2 commits into
Conversation
michaelotis
left a comment
michaelotis
left a comment
There was a problem hiding this comment.
Good use of shared_ptr to allow for wallet object destruction
jc23424
left a comment
jc23424
left a comment
There was a problem hiding this comment.
I think best to follow these style guidelines going forward https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md
so m_pwallet instead of pwallet_
| @@ -44,6 +48,7 @@ AsyncRPCOperation::AsyncRPCOperation(const AsyncRPCOperation& o) : | |||
| } | |||
|
|
|||
| AsyncRPCOperation& AsyncRPCOperation::operator=( const AsyncRPCOperation& other ) { | |||
There was a problem hiding this comment.
this doesn't really feel like something that should be copy assignable - i.e. what happens if i take a copy and cancel the copy?
it should be move assignable - but i don't see any reason why the default move assignment wouldn't work. if you just delete this operator and replace any assignments with a move?
i.e. AsyncRPCOperation x = y; becomes AsyncRPCOperation x = std::move(y)
There was a problem hiding this comment.
Good insight; agreed
4 participants
This is the better define the lifecycle and ownership model of
AsyncRPCOperationandCWallets.Also,
pwalletMain->pwallet_.