Block unsafe std::string fs::path conversion copy_file calls #24026

1. 
   hebasto commented at 10:04 pm on January 10, 2022: member

   This PR is an optional prerequisite for bitcoin/bitcoin#20744 “Use std::filesystem. Remove Boost Filesystem & System” which:

   • makes further code changes safer
   • prevents some test failures on native Windows
2. refactor: Block unsafe std::string fs::path conversion copy_file calls

   There is no change in behavior. This just helps prepare for the
   transition from boost::filesystem to std::filesystem by avoiding
   copy_file calls that will be unsafe after the transition to
   std::filesystem to due lack of a boost::filesystem::path::imbue
   equivalent and inability to set a predictable locale.

   213172c734
3. Change type of `backup_file` parameter in RestoreWallet/restoreWallet

   `fs::path` looks more native than `std::string` for a parameter which
   represents a backup file. This change eliminates back-and-forth type
   conversions.

   3a45dc36a6
4. DrahtBot added the label interfaces on Jan 10, 2022
5. DrahtBot added the label RPC/REST/ZMQ on Jan 10, 2022
6. DrahtBot added the label Wallet on Jan 10, 2022
7. in src/interfaces/wallet.h:330 in 3a45dc36a6

   326@@ -326,7 +327,7 @@ class WalletLoader : public ChainClient
   327    virtual std::string getWalletDir() = 0;
   328 
   329    //! Restore backup wallet
   330-   virtual std::unique_ptr<Wallet> restoreWallet(const std::string& backup_file, const std::string& wallet_name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0;
   331+   virtual std::unique_ptr<Wallet> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0;
   

   ---

   ---

   

   ryanofsky commented at 10:12 pm on January 10, 2022:

   In commit “Change type of backup_file parameter in RestoreWallet/restoreWallet” (3a45dc36a663ea67f13e7d5c413518b9385ec594)

   Hmm, interesting that this method doesn’t seem to be called anywhere. I guess whatever PR which will use it will also need to be updated.

   ---

   

   achow101 commented at 10:39 pm on January 10, 2022:

   https://github.com/bitcoin-core/gui/pull/471 is the PR using it.
8. ryanofsky approved
9. 
   ryanofsky commented at 10:13 pm on January 10, 2022: member

   Code review ACK 3a45dc36a663ea67f13e7d5c413518b9385ec594. Looks great! Thanks for debugging and fixing this and making #20744 smaller!
10. 
    DrahtBot commented at 2:06 am on January 11, 2022: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #20744 (Use std::filesystem. Remove Boost Filesystem & System by fanquake)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

11. fanquake merged this on Jan 11, 2022
12. fanquake closed this on Jan 11, 2022

    ---

13. 
    achow101 commented at 5:48 pm on January 11, 2022: member

    post-merge ACK 3a45dc36a663ea67f13e7d5c413518b9385ec594
14. hebasto deleted the branch on Jan 11, 2022
15. sidhujag referenced this in commit 7f43055fac on Jan 12, 2022
16. DrahtBot locked this on Jan 11, 2023

Contributors
hebasto ryanofsky achow101 DrahtBot

Labels
Wallet RPC/REST/ZMQ interfaces