fs: consistently use fsbridge:: for ifstream / ofstream #24167
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Part of bitcoin#20744, but this can be done now, and will simplify the diff.
This is needed to prevent compilation failures once boost is removed, however is still correct to include now, and reduces the diff in bitcoin#20744. <string> is extracted from the defines because it is used for Windows and non-Windows code, i.e get_filesystem_error_message().
Member
|
Concept ACK. |
maflcko
approved these changes
Jan 26, 2022
Member
maflcko
left a comment
maflcko
left a comment
There was a problem hiding this comment.
ACK 5e8975e 🏕
Show signature
Signature:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
ACK 5e8975e2694c3178ae73deb28986e1fb5466147e 🏕
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUg91Av+IL93ayKaf1O3lEaZkky3cz+mduqUCE8hGE6UQo9NSM47OvHEeTRPVK0H
DHTpZidWXOJbzuBvKhs7YLOm4xgYN5Wx2/UDtFHIMFTQynsxZjPYDILWPNXjUBma
pwNS59KsWZO1iZNqBaG+gHUA2zGIbVDLCP+wY/HbeTDIOr+Sr27Y3lxha2AUDYKr
O1J+Hxre7S9nDSp+x5i9XcL6qpw0xs8nzqweRYd1gBUGkEj1FzpcYBgh2QCbuOqD
idxw9dA8f3m1Y+1P/kpSKw3Kzvc/Ce18gOmCzUMKpXEfwfLsbPOXG468xviJpc5Y
1EaFsrZHUMOpz6VRFZ+bl/xfH3vCdCMZY9XNyV4HzG8Rnz7JHzJS4qXmx8eILKK0
V9uc7sseLDJ9ttX1Vy5/hyWDOnSnu8YPpt3f4gPeGA79+sjBMOMG5451hSzazNP4
4yQs8uUVEWH/j97XQQFdD6ldFQtrD2dU4zb3mxnA7V8xPZmMjaspJE4rCZveytas
ty+5S2DM
=nQnW
-----END PGP SIGNATURE-----
| std::string filename = strInput.substr(pos + 1, std::string::npos); | ||
|
|
||
| FILE *f = fopen(filename.c_str(), "r"); | ||
| FILE *f = fsbridge::fopen(filename.c_str(), "r"); |
Member
There was a problem hiding this comment.
unrelated question for follow-up: Why is this safe? Shouldn't this use PathFromString?
Member
|
It's good to be consistent with this. |
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jan 28, 2022
…fstream 5e8975e fs: consistently use fsbridge for fopen() (fanquake) 486261d fs: add missing <cassert> include (fanquake) 21f781a fs: consistently use fsbridge for {i,o}fstream (fanquake) Pull request description: These changes are part of bitcoin#20744, but are also ok to do now, and reduce the diff in that PR. See commit messages for details. Revived from bitcoin#23857. ACKs for top commit: laanwj: Code review ACK 5e8975e MarcoFalke: ACK 5e8975e 🏕 Tree-SHA512: ee2dc857ce2479b39b65615e689f934b962e580299b0e7a0c6361633402b0d61e6e4479f41f6480e2c46101264d93f330b8f7b57e56df95a7f77e046a4e44697
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Sep 20, 2022
Summary:
> fs: consistently use fsbridge for {i,o}fstream
> fs: add missing <cassert> include
>
> This is needed to prevent compilation failures once boost is removed,
> however is still correct to include now, and reduces the diff in #20744.
>
> <string> is extracted from the defines because it is used for Windows
> and non-Windows code, i.e get_filesystem_error_message().
> fs: consistently use fsbridge for fopen()
This is a backport of [[bitcoin/bitcoin#24167 | core#24167]]
Depends on D11984
Notes:
- the psbtoperationsdialog.cpp and fuzz.cpp changes are not applicable due to missing backports
- the walletframe.cpp changes are in walletview.cpp because of missing backports (core-gui#399)
Test Plan: `ninja all check-all`
Reviewers: #bitcoin_abc, sdulfari, Fabien
Reviewed By: #bitcoin_abc, sdulfari, Fabien
Subscribers: Fabien, sdulfari
Differential Revision: https://reviews.bitcoinabc.org/D11975
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These changes are part of #20744, but are also ok to do now, and reduce the diff in that PR. See commit messages for details. Revived from #23857.