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.

Sign up for GitHub

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

Jump to bottom

Let's use the Win32 API more precisely #147

Closed
wants to merge 2 commits into from
Closed

Let's use the Win32 API more precisely #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
54fff4e
mingw: get pw_name in UTF-8 format
dscho Feb 26, 2019
0c9d2ae
mingw: use Unicode functions explicitly
dscho Nov 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions compat/mingw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1407,7 +1407,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
do_unset_environment_variables();
Copy link

Choose a reason for hiding this comment

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

On the Git mailing list, Eric Sunshine wrote (reply to this):

On Thu, Jun 27, 2019 at 5:37 AM Johannes Schindelin via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> Many Win32 API functions actually exist in two variants: one with
> the `A` suffix that takes ANSI parameters (`char *` or `const char *`)
> and one with the `W` suffix that takes Unicode parameters (`wchar_t *`
> or `const wchar_t *`).
>
> The ANSI variant assumes that the strings are encoded according to
> whatever is the current locale. This is not what Git wants to use on
> Windows: we assume that `char *` variables point to strings encoded in
> UTF-8.
>
> There is a pseudo UTF-8 locale on Windows, but it does not work
> as one might expect.

What does "does not work as one might expect" mean? The reader is left
hanging, not knowing why or how the UTF-8 locale on Windows is
undesirable.

> In addition, if we overrode the user's locale, that
> would modify the behavior of programs spawned by Git (such as editors,
> difftools, etc), therefore we cannot use that pseudo locale.
>
> Further, it is actually highly encouraged to use the Unicode versions
> instead of the ANSI versions, so let's do precisely that.
>
> Note: when calling the Win32 API functions _without_ any suffix, it
> depends whether the `UNICODE` constant is defined before the relevant
> headers are #include'd. Without that constant, the ANSI variants are
> used. Let's be explicit and avoid that ambiguity.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Copy link

Choose a reason for hiding this comment

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

On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Eric,

On Thu, 27 Jun 2019, Eric Sunshine wrote:

> On Thu, Jun 27, 2019 at 5:37 AM Johannes Schindelin via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> > Many Win32 API functions actually exist in two variants: one with
> > the `A` suffix that takes ANSI parameters (`char *` or `const char *`)
> > and one with the `W` suffix that takes Unicode parameters (`wchar_t *`
> > or `const wchar_t *`).
> >
> > The ANSI variant assumes that the strings are encoded according to
> > whatever is the current locale. This is not what Git wants to use on
> > Windows: we assume that `char *` variables point to strings encoded in
> > UTF-8.
> >
> > There is a pseudo UTF-8 locale on Windows, but it does not work
> > as one might expect.
>
> What does "does not work as one might expect" mean? The reader is left
> hanging, not knowing why or how the UTF-8 locale on Windows is
> undesirable.

Should I really bore the reader with half-details? At least one of those
behaviors was reported as an unbootable Windows 7, but some others could
not reproduce.

I really did not want to write a novel here that is 1) largely irrelevant
and 2) would take way too long to write because I forgot most of the
details and would have to look them up again.

Hopefully we can do without the full story here, as even one report that
it does not work as expected is enough to make this an unfeasible option
for us, and that's that as far as I am concerned.

Ciao,
Dscho

>
> > In addition, if we overrode the user's locale, that
> > would modify the behavior of programs spawned by Git (such as editors,
> > difftools, etc), therefore we cannot use that pseudo locale.
> >
> > Further, it is actually highly encouraged to use the Unicode versions
> > instead of the ANSI versions, so let's do precisely that.
> >
> > Note: when calling the Win32 API functions _without_ any suffix, it
> > depends whether the `UNICODE` constant is defined before the relevant
> > headers are #include'd. Without that constant, the ANSI variants are
> > used. Let's be explicit and avoid that ambiguity.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>


/* Determine whether or not we are associated to a console */
cons = CreateFile("CONOUT$", GENERIC_WRITE,
cons = CreateFileW(L"CONOUT$", GENERIC_WRITE,
FILE_SHARE_WRITE, NULL, OPEN_EXISTING,
FILE_ATTRIBUTE_NORMAL, NULL);
if (cons == INVALID_HANDLE_VALUE) {
Expand Down Expand Up @@ -1946,13 +1946,19 @@ struct passwd *getpwuid(int uid)
static unsigned initialized;
dscho marked this conversation as resolved.
Show resolved Hide resolved
static char user_name[100];
static struct passwd *p;
wchar_t buf[100];
DWORD len;

if (initialized)
return p;

len = sizeof(user_name);
if (!GetUserName(user_name, &len)) {
len = sizeof(buf);
if (!GetUserNameW(buf, &len)) {
initialized = 1;
return NULL;
}

if (xwcstoutf(user_name, buf, sizeof(user_name)) < 0) {
initialized = 1;
return NULL;
}
Expand Down
2 changes: 1 addition & 1 deletion compat/poll/poll.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ win32_compute_revents (HANDLE h, int *p_sought)
if (!once_only)
{
NtQueryInformationFile = (PNtQueryInformationFile)
GetProcAddress (GetModuleHandle ("ntdll.dll"),
GetProcAddress (GetModuleHandleW (L"ntdll.dll"),
"NtQueryInformationFile");
once_only = TRUE;
}
Expand Down
10 changes: 6 additions & 4 deletions compat/winansi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ int winansi_isatty(int fd)
void winansi_init(void)
{
int con1, con2;
char name[32];
wchar_t name[32];

/* check if either stdout or stderr is a console output screen buffer */
con1 = is_console(1);
Expand All @@ -619,13 +619,15 @@ void winansi_init(void)
}

/* create a named pipe to communicate with the console thread */
xsnprintf(name, sizeof(name), "\\\\.\\pipe\\winansi%lu", GetCurrentProcessId());
hwrite = CreateNamedPipe(name, PIPE_ACCESS_OUTBOUND,
if (swprintf(name, ARRAY_SIZE(name) - 1, L"\\\\.\\pipe\\winansi%lu",
GetCurrentProcessId()) < 0)
die("Could not initialize winansi pipe name");
hwrite = CreateNamedPipeW(name, PIPE_ACCESS_OUTBOUND,
PIPE_TYPE_BYTE | PIPE_WAIT, 1, BUFFER_SIZE, 0, 0, NULL);
if (hwrite == INVALID_HANDLE_VALUE)
die_lasterr("CreateNamedPipe failed");

hread = CreateFile(name, GENERIC_READ, 0, NULL, OPEN_EXISTING, 0, NULL);
hread = CreateFileW(name, GENERIC_READ, 0, NULL, OPEN_EXISTING, 0, NULL);
if (hread == INVALID_HANDLE_VALUE)
die_lasterr("CreateFile for named pipe failed");

Expand Down