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

Wallet: use unique_ptr for WalletImpl members #4417

Closed
wants to merge 1 commit into from

Conversation

coneiric
Copy link
Contributor

Use unique_ptr to manage WalletImpl internals, rather than raw pointers.

Use unique_ptr to manage WalletImpl internals, rather than raw
pointers.
Copy link
Contributor

@fluffypony fluffypony left a comment

Choose a reason for hiding this comment

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

Reviewed

fluffypony added a commit that referenced this pull request Sep 29, 2018
a21da90 Wallet: use unique_ptr for WalletImpl members (oneiric)
fluffypony added a commit that referenced this pull request Sep 29, 2018
a21da90 Wallet: use unique_ptr for WalletImpl members (oneiric)
@radfish
Copy link
Contributor

radfish commented Sep 29, 2018

This patch broke the build on x86_64 Arch boost 1.68.0-1:

/home/redfish/dev/monero-git/src/monero/src/wallet/api/wallet.cpp: In constructor ‘Monero::WalletImpl::WalletImpl(Monero::NetworkType, uint64_t)’:
/home/redfish/dev/monero-git/src/monero/src/wallet/api/wallet.cpp:379:21: error: ‘make_unique’ is not a member of ‘std’
     m_wallet = std::make_unique<tools::wallet2>(static_cast<cryptonote::network_type>(nettype), kdf_rounds, true);
                     ^~~~~~~~~~~
/home/redfish/dev/monero-git/src/monero/src/wallet/api/wallet.cpp:379:21: note: ‘std::make_unique’ is defined in header ‘<memory>’; did you forget to ‘#include <memory>’?
/home/redfish/dev/monero-git/src/monero/src/wallet/api/wallet.cpp:45:1:
+#include <memory>
 #include <sstream>

Adding #include <memory> did not help. Reverting this patch fixes the build, fwiw.

@fluffypony fluffypony closed this Sep 30, 2018
@iDunk5400
Copy link
Contributor

This patch broke the build with -D BUILD_GUI_DEPS on Ubuntu 16.04 (GCC 5.4.0).

/home/user/monero/src/wallet/api/wallet.cpp: In constructor ‘Monero::WalletImpl::WalletImpl(Monero::NetworkType, uint64_t)’:
/home/user/monero/src/wallet/api/wallet.cpp:379:16: error: ‘make_unique’ is not a member of ‘std’
     m_wallet = std::make_unique<tools::wallet2>(static_cast<cryptonote::network_type>(nettype), kdf_rounds, true);
                ^
/home/user/monero/src/wallet/api/wallet.cpp:379:47: error: expected primary-expression before ‘>’ token
     m_wallet = std::make_unique<tools::wallet2>(static_cast<cryptonote::network_type>(nettype), kdf_rounds, true);
                                               ^
/home/user/monero/src/wallet/api/wallet.cpp:379:97: error: left operand of comma operator has no effect [-Werror=unused-value]
     m_wallet = std::make_unique<tools::wallet2>(static_cast<cryptonote::network_type>(nettype), kdf_rounds, true);
                                                                                                 ^
/home/user/monero/src/wallet/api/wallet.cpp:379:109: error: right operand of comma operator has no effect [-Werror=unused-value]
     m_wallet = std::make_unique<tools::wallet2>(static_cast<cryptonote::network_type>(nettype), kdf_rounds, true);
                                                                                                             ^
/home/user/monero/src/wallet/api/wallet.cpp:380:17: error: ‘make_unique’ is not a member of ‘std’
     m_history = std::make_unique<TransactionHistoryImpl>(this);
                 ^
/home/user/monero/src/wallet/api/wallet.cpp:380:56: error: expected primary-expression before ‘>’ token
     m_history = std::make_unique<TransactionHistoryImpl>(this);
                                                        ^
/home/user/monero/src/wallet/api/wallet.cpp:381:25: error: ‘make_unique’ is not a member of ‘std’
     m_wallet2Callback = std::make_unique<Wallet2CallbackImpl>(this);
                         ^
/home/user/monero/src/wallet/api/wallet.cpp:381:61: error: expected primary-expression before ‘>’ token
     m_wallet2Callback = std::make_unique<Wallet2CallbackImpl>(this);
                                                             ^
/home/user/monero/src/wallet/api/wallet.cpp:382:41: error: no matching function for call to ‘tools::wallet2::callback(std::unique_ptr<Monero::Wallet2CallbackImpl>&)’
     m_wallet->callback(m_wallet2Callback);
                                         ^
In file included from /home/user/monero/src/wallet/api/wallet.h:35:0,
                 from /home/user/monero/src/wallet/api/wallet.cpp:32:
/home/user/monero/src/wallet/wallet2.h:648:25: note: candidate: tools::i_wallet2_callback* tools::wallet2::callback() const
     i_wallet2_callback* callback() const { return m_callback; }
                         ^
/home/user/monero/src/wallet/wallet2.h:648:25: note:   candidate expects 0 arguments, 1 provided
/home/user/monero/src/wallet/wallet2.h:649:10: note: candidate: void tools::wallet2::callback(tools::i_wallet2_callback*)
     void callback(i_wallet2_callback* callback) { m_callback = callback; }
          ^
/home/user/monero/src/wallet/wallet2.h:649:10: note:   no known conversion for argument 1 from ‘std::unique_ptr<Monero::Wallet2CallbackImpl>’ to ‘tools::i_wallet2_callback*’
/home/user/monero/src/wallet/api/wallet.cpp:385:21: error: ‘make_unique’ is not a member of ‘std’
     m_addressBook = std::make_unique<AddressBookImpl>(this);
                     ^
/home/user/monero/src/wallet/api/wallet.cpp:385:53: error: expected primary-expression before ‘>’ token
     m_addressBook = std::make_unique<AddressBookImpl>(this);
                                                     ^
/home/user/monero/src/wallet/api/wallet.cpp:386:20: error: ‘make_unique’ is not a member of ‘std’
     m_subaddress = std::make_unique<SubaddressImpl>(this);
                    ^
/home/user/monero/src/wallet/api/wallet.cpp:386:51: error: expected primary-expression before ‘>’ token
     m_subaddress = std::make_unique<SubaddressImpl>(this);
                                                   ^
/home/user/monero/src/wallet/api/wallet.cpp:387:27: error: ‘make_unique’ is not a member of ‘std’
     m_subaddressAccount = std::make_unique<SubaddressAccountImpl>(this);
                           ^
/home/user/monero/src/wallet/api/wallet.cpp:387:65: error: expected primary-expression before ‘>’ token
     m_subaddressAccount = std::make_unique<SubaddressAccountImpl>(this);
                                                                 ^
/home/user/monero/src/wallet/api/wallet.cpp: In member function ‘virtual bool Monero::WalletImpl::finalizeMultisig(const std::vector<std::__cxx11::basic_string<char> >&)’:
/home/user/monero/src/wallet/api/wallet.cpp:1187:45: error: cannot convert ‘std::unique_ptr<tools::wallet2>’ to ‘const tools::wallet2*’ for argument ‘1’ to ‘void Monero::{anonymous}::checkMultisigWalletNotReady(const tools::wallet2*)’
         checkMultisigWalletNotReady(m_wallet);
                                             ^
/home/user/monero/src/wallet/api/wallet.cpp: In member function ‘virtual bool Monero::WalletImpl::exportMultisigImages(std::__cxx11::string&)’:
/home/user/monero/src/wallet/api/wallet.cpp:1205:42: error: cannot convert ‘std::unique_ptr<tools::wallet2>’ to ‘const tools::wallet2*’ for argument ‘1’ to ‘void Monero::{anonymous}::checkMultisigWalletReady(const tools::wallet2*)’
         checkMultisigWalletReady(m_wallet);
                                          ^
/home/user/monero/src/wallet/api/wallet.cpp: In member function ‘virtual size_t Monero::WalletImpl::importMultisigImages(const std::vector<std::__cxx11::basic_string<char> >&)’:
/home/user/monero/src/wallet/api/wallet.cpp:1221:42: error: cannot convert ‘std::unique_ptr<tools::wallet2>’ to ‘const tools::wallet2*’ for argument ‘1’ to ‘void Monero::{anonymous}::checkMultisigWalletReady(const tools::wallet2*)’
         checkMultisigWalletReady(m_wallet);
                                          ^
/home/user/monero/src/wallet/api/wallet.cpp: In member function ‘virtual bool Monero::WalletImpl::hasMultisigPartialKeyImages() const’:
/home/user/monero/src/wallet/api/wallet.cpp:1249:42: error: cannot convert ‘const std::unique_ptr<tools::wallet2>’ to ‘const tools::wallet2*’ for argument ‘1’ to ‘void Monero::{anonymous}::checkMultisigWalletReady(const tools::wallet2*)’
         checkMultisigWalletReady(m_wallet);
                                          ^
/home/user/monero/src/wallet/api/wallet.cpp: In member function ‘virtual Monero::PendingTransaction* Monero::WalletImpl::restoreMultisigTransaction(const string&)’:
/home/user/monero/src/wallet/api/wallet.cpp:1263:42: error: cannot convert ‘std::unique_ptr<tools::wallet2>’ to ‘const tools::wallet2*’ for argument ‘1’ to ‘void Monero::{anonymous}::checkMultisigWalletReady(const tools::wallet2*)’
         checkMultisigWalletReady(m_wallet);
                                          ^
cc1plus: all warnings being treated as errors
src/wallet/api/CMakeFiles/obj_wallet_api.dir/build.make:62: recipe for target 'src/wallet/api/CMakeFiles/obj_wallet_api.dir/wallet.cpp.o' failed
make[3]: *** [src/wallet/api/CMakeFiles/obj_wallet_api.dir/wallet.cpp.o] Error 1
make[3]: *** Waiting for unfinished jobs....

Including <memory> makes no difference at this point, until this change in CMakeLists.txt

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 9c11402..491de36 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -595,7 +595,7 @@ else()
     set(STATIC_ASSERT_FLAG "-Dstatic_assert=_Static_assert")
   endif()

-  try_compile(STATIC_ASSERT_CPP_RES "${CMAKE_CURRENT_BINARY_DIR}/static-assert" "${CMAKE_CURRENT_SOURCE_DIR}/cmake/test-static-assert.cpp" COMPILE_DEFINITIONS "-std=c++11")
+  try_compile(STATIC_ASSERT_CPP_RES "${CMAKE_CURRENT_BINARY_DIR}/static-assert" "${CMAKE_CURRENT_SOURCE_DIR}/cmake/test-static-assert.cpp" COMPILE_DEFINITIONS "-std=c++14")
   if(STATIC_ASSERT_CPP_RES)
     set(STATIC_ASSERT_CPP_FLAG "")
   else()
@@ -674,7 +674,7 @@ else()
   message(STATUS "Using linker security hardening flags: ${LD_SECURITY_FLAGS}")

   set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=c11 -D_GNU_SOURCE ${MINGW_FLAG} ${STATIC_ASSERT_FLAG} ${WARNINGS} ${C_WARNINGS} ${COVERAGE_FLAGS} ${PIC_FLAG} ${C_SECURITY_FLAGS}")
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -D_GNU_SOURCE ${MINGW_FLAG} ${STATIC_ASSERT_CPP_FLAG} ${WARNINGS} ${CXX_WARNINGS} ${COVERAGE_FLAGS} ${PIC_FLAG} ${CXX_SECURITY_FLAGS}")
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++14 -D_GNU_SOURCE ${MINGW_FLAG} ${STATIC_ASSERT_CPP_FLAG} ${WARNINGS} ${CXX_WARNINGS} ${COVERAGE_FLAGS} ${PIC_FLAG} ${CXX_SECURITY_FLAGS}")
   set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${LD_SECURITY_FLAGS}")

   # With GCC 6.1.1 the compiled binary malfunctions due to aliasing. Until that

After that, the build still fails with

/home/user/monero/src/wallet/api/wallet.cpp: In constructor ‘Monero::WalletImpl::WalletImpl(Monero::NetworkType, uint64_t)’:
/home/user/monero/src/wallet/api/wallet.cpp:382:41: error: no matching function for call to ‘tools::wallet2::callback(std::unique_ptr<Monero::Wallet2CallbackImpl>&)’
     m_wallet->callback(m_wallet2Callback);
                                         ^
In file included from /home/user/monero/src/wallet/api/wallet.h:35:0,
                 from /home/user/monero/src/wallet/api/wallet.cpp:32:
/home/user/monero/src/wallet/wallet2.h:648:25: note: candidate: tools::i_wallet2_callback* tools::wallet2::callback() const
     i_wallet2_callback* callback() const { return m_callback; }
                         ^
/home/user/monero/src/wallet/wallet2.h:648:25: note:   candidate expects 0 arguments, 1 provided
/home/user/monero/src/wallet/wallet2.h:649:10: note: candidate: void tools::wallet2::callback(tools::i_wallet2_callback*)
     void callback(i_wallet2_callback* callback) { m_callback = callback; }
          ^
/home/user/monero/src/wallet/wallet2.h:649:10: note:   no known conversion for argument 1 from ‘std::unique_ptr<Monero::Wallet2CallbackImpl>’ to ‘tools::i_wallet2_callback*’
/home/user/monero/src/wallet/api/wallet.cpp: In member function ‘virtual bool Monero::WalletImpl::finalizeMultisig(const std::vector<std::__cxx11::basic_string<char> >&)’:
/home/user/monero/src/wallet/api/wallet.cpp:1187:45: error: cannot convert ‘std::unique_ptr<tools::wallet2>’ to ‘const tools::wallet2*’ for argument ‘1’ to ‘void Monero::{anonymous}::checkMultisigWalletNotReady(const tools::wallet2*)’
         checkMultisigWalletNotReady(m_wallet);
                                             ^
/home/user/monero/src/wallet/api/wallet.cpp: In member function ‘virtual bool Monero::WalletImpl::exportMultisigImages(std::__cxx11::string&)’:
/home/user/monero/src/wallet/api/wallet.cpp:1205:42: error: cannot convert ‘std::unique_ptr<tools::wallet2>’ to ‘const tools::wallet2*’ for argument ‘1’ to ‘void Monero::{anonymous}::checkMultisigWalletReady(const tools::wallet2*)’
         checkMultisigWalletReady(m_wallet);
                                          ^
/home/user/monero/src/wallet/api/wallet.cpp: In member function ‘virtual size_t Monero::WalletImpl::importMultisigImages(const std::vector<std::__cxx11::basic_string<char> >&)’:
/home/user/monero/src/wallet/api/wallet.cpp:1221:42: error: cannot convert ‘std::unique_ptr<tools::wallet2>’ to ‘const tools::wallet2*’ for argument ‘1’ to ‘void Monero::{anonymous}::checkMultisigWalletReady(const tools::wallet2*)’
         checkMultisigWalletReady(m_wallet);
                                          ^
/home/user/monero/src/wallet/api/wallet.cpp: In member function ‘virtual bool Monero::WalletImpl::hasMultisigPartialKeyImages() const’:
/home/user/monero/src/wallet/api/wallet.cpp:1249:42: error: cannot convert ‘const std::unique_ptr<tools::wallet2>’ to ‘const tools::wallet2*’ for argument ‘1’ to ‘void Monero::{anonymous}::checkMultisigWalletReady(const tools::wallet2*)’
         checkMultisigWalletReady(m_wallet);
                                          ^
/home/user/monero/src/wallet/api/wallet.cpp: In member function ‘virtual Monero::PendingTransaction* Monero::WalletImpl::restoreMultisigTransaction(const string&)’:
/home/user/monero/src/wallet/api/wallet.cpp:1263:42: error: cannot convert ‘std::unique_ptr<tools::wallet2>’ to ‘const tools::wallet2*’ for argument ‘1’ to ‘void Monero::{anonymous}::checkMultisigWalletReady(const tools::wallet2*)’
         checkMultisigWalletReady(m_wallet);
                                          ^
src/wallet/api/CMakeFiles/obj_wallet_api.dir/build.make:62: recipe for target 'src/wallet/api/CMakeFiles/obj_wallet_api.dir/wallet.cpp.o' failed
make[3]: *** [src/wallet/api/CMakeFiles/obj_wallet_api.dir/wallet.cpp.o] Error 1
make[3]: *** Waiting for unfinished jobs....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants