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

utils: Use _wfopen and _wfreopen on Windows #13866

Merged
merged 1 commit into from Sep 11, 2018
Merged

Conversation

@ken2812221
Copy link
Contributor

@ken2812221 ken2812221 commented Aug 3, 2018

The fopen function does not support unicode filename on Windows, so use Windows specific function do deal with it.

@Empact
Copy link
Member

@Empact Empact commented Aug 3, 2018

@ken2812221
Copy link
Contributor Author

@ken2812221 ken2812221 commented Aug 3, 2018

@Empact They are the same if we ignore the errno.

@DrahtBot
Copy link
Contributor

@DrahtBot DrahtBot commented Aug 3, 2018

No more conflicts as of last run.

#ifdef WIN32
std::wstring Utf8ToWide(const std::string& utf8_string)
{
size_t size = MultiByteToWideChar(CP_UTF8, 0, &*utf8_string.begin(), utf8_string.size(), nullptr, 0);
Copy link
Contributor

@donaloconnor donaloconnor Aug 3, 2018

I find the &* unnecessarily complicated. Can you use utf8_string.data() or even the more usual c_str() instead that gives a direct pointer back.

Copy link
Contributor Author

@ken2812221 ken2812221 Aug 3, 2018

This avoid the unnecessary copy, &*utf8_string.begin() point to where the exact string data are. c_str() and data() is a copy of the string data.

Copy link
Contributor

@donaloconnor donaloconnor Aug 4, 2018

Not sure what implementation you are referring to but c_str and data have always returned the pointer to the internal data. Not a copy. Anyway it was just a nitpick. Thanks.

size_t size = MultiByteToWideChar(CP_UTF8, 0, &*utf8_string.begin(), utf8_string.size(), nullptr, 0);
std::wstring wide_string(size, 0);
size = MultiByteToWideChar(CP_UTF8, 0, &*utf8_string.begin(), utf8_string.size(), &*wide_string.begin(), wide_string.size());
assert(size == wide_string.size());
Copy link
Contributor

@donaloconnor donaloconnor Aug 3, 2018

Is this assert necessary? MultiByteToWideChar won't change the size of the string.

Copy link
Contributor Author

@ken2812221 ken2812221 Aug 3, 2018

They won't change, so that's why I use assert instead of a bunch of error handling.

Copy link
Contributor

@donaloconnor donaloconnor Aug 4, 2018

I didn't spot the size= part before the asset. Sorry :)

src/fs.cpp Outdated
@@ -1,15 +1,24 @@
#include <fs.h>
#include <utilstrencodings.h>
Copy link
Contributor

@practicalswift practicalswift Aug 4, 2018

nit: Make this include conditional on WIN32?

@ken2812221 ken2812221 force-pushed the fopen-windows branch 5 times, most recently from 5a5e024 to a8c90e3 Aug 5, 2018
@ken2812221
Copy link
Contributor Author

@ken2812221 ken2812221 commented Aug 5, 2018

Update: use std codecvt instead of custom converter.

@ken2812221 ken2812221 changed the title utils: Use _wfopen and _wreopen on Windows utils: Use _wfopen and _wfreopen on Windows Aug 16, 2018
@laanwj laanwj added this to Blockers in High-priority for review Aug 16, 2018
@laanwj laanwj removed this from Blockers in High-priority for review Aug 30, 2018
The fopen function does not support unicode filename on Windows, so use Windows specific function do deal with it
Copy link
Contributor

@ryanofsky ryanofsky left a comment

utACK b9babc8

@laanwj
Copy link
Member

@laanwj laanwj commented Sep 11, 2018

utACK b9babc8

@laanwj laanwj merged commit b9babc8 into bitcoin:master Sep 11, 2018
2 checks passed
laanwj added a commit that referenced this issue Sep 11, 2018
b9babc8 utils: Use _wfopen and _wreopen on Windows (Chun Kuan Lee)

Pull request description:

  The fopen function does not support unicode filename on Windows, so use Windows specific function do deal with it.

Tree-SHA512: 4dcf14dcf9ec6307b9fdf95404e5b6b6b3df640949fd4b0c4ac7fecf8ea03a64fa25285fc319c4ff8a28e586eee106f1861116c181694955497402b2bf575f22
@ken2812221 ken2812221 deleted the fopen-windows branch Sep 11, 2018
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this issue Nov 10, 2020
Summary:
The fopen function does not support unicode filename on Windows, so use Windows specific function do deal with it

This is a backport of Core [[bitcoin/bitcoin#13866 | PR13866]]

Test Plan:
`ninja && ninja check`

Use the build bot to run tests on windows.

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D8343
kittywhiskers added a commit to kittywhiskers/dash that referenced this issue May 19, 2021
kittywhiskers added a commit to kittywhiskers/dash that referenced this issue May 19, 2021
kittywhiskers added a commit to kittywhiskers/dash that referenced this issue May 20, 2021
kittywhiskers added a commit to kittywhiskers/dash that referenced this issue May 20, 2021
kittywhiskers added a commit to kittywhiskers/dash that referenced this issue May 20, 2021
kittywhiskers added a commit to kittywhiskers/dash that referenced this issue May 20, 2021
random-zebra added a commit to PIVX-Project/PIVX that referenced this issue Aug 5, 2021
63e0be6 [Remove] By-pass logprint-scanner restriction. (furszy)
280ced3 utils: Fix broken Windows filelock (Chun Kuan Lee)
be89860 utils: Convert Windows args to utf-8 string (Chun Kuan Lee)
e8cfa6e Call unicode API on Windows (Chun Kuan Lee)
1a02a8a tests: Add test case for std::ios_base::ate (Chun Kuan Lee)
2e57cd4 Move boost/std fstream to fsbridge (furszy)
9d8bcd4 utils: Add fsbridge fstream function wrapper (Chun Kuan Lee)
d59d48d utils: Convert fs error messages from multibyte to utf-8 (ken2812221)
9ef58cc Logging: use "fmterr" variable name for errors instead of general "e" that can be used by any other function. (furszy)
dd94241 utils: Use _wfopen and _wreopen on Windows (Chun Kuan Lee)
3993641 add unicode compatible file_lock for Windows (Chun Kuan Lee)
48349f8 Provide relevant error message if datadir is not writable. (murrayn)

Pull request description:

  As the software is currently using the ANSI encoding on Windows, the user's language settings could affect the proper functioning of the node/wallet, to the point of not be able to open some non-ASCII name files and directories.

  This solves the Windows encoding issues, completing the entire bitcoin#13869 work path (and some other required backports). Enabling for example users that use non-english characters in directories and file names to be accepted.

  Backported PRs:
  * bitcoin#12422.
  * bitcoin#12630.
  * bitcoin#13862.
  * bitcoin#13866.
  * bitcoin#13877.
  * bitcoin#13878.
  * bitcoin#13883.
  * bitcoin#13884.
  * bitcoin#13886.
  * bitcoin#13888.
  * bitcoin#14192.
  * bitcoin#13734.
  * bitcoin#14426.

  This is built on top of other two PRs that i have open #2423 and #2369.
  Solves old issues #940 and #2163.

  TODO:
  * Backport `assert_start_raises_init_error` and `ErrorMatch` in TestNode` (bitcoin#12718)

ACKs for top commit:
  Fuzzbawls:
    ACK 63e0be6
  random-zebra:
    ACK 63e0be6 and merging...

Tree-SHA512: cb1f7c23abb5b7b3af50bba18652cc2cad93fd7c2fca9c16ffd3fee34c4c152a3b666dfa87fe6b44c430064dcdee4367144dcb4a41203c91b0173b805bdb3d7d
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants