{"thread":{"id":"51395","subject":"[PATCH 0/2] Let's use the Win32 API more precisely","startedAt":"2019-06-27T09:37:21Z","lastAt":"2019-07-04T21:54:32Z","messageCount":8,"participants":["Johannes Schindelin via GitGitGadget","Eric Sunshine","Johannes Schindelin","Beat Bolli"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"378146","messageId":"pull.147.git.gitgitgadget@gmail.com","threadId":"51395","inReplyTo":null,"subject":"[PATCH 0/2] Let's use the Win32 API more precisely","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-06-27T09:37:17Z","receivedAt":"2019-06-27T09:37:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"For many Win32 functions, there actually exist two variants: one that takes \nconst char * (\"ANSI\", meaning the current code page) and wchar_t * \n(\"Unicode\", i.e. UTF-16, at least for all practical matters).\n\nThese functions have \"A\" and \"W\" suffixes, respectively, e.g. \nGetFileAttributesW(). The symbols without this suffix are #defined to the \n*W() versions if the constant UNICODE is defined before including the\nWindows headers, and to *A() otherwise.\n\nLet's not rely on this constant, but explicitly say what we want: we want\nthe Unicode versions, as they seem to be used by the ANSI flavor anyway.\n\nJohannes Schindelin (2):\n mingw: get pw_name in UTF-8 format\n mingw: use Unicode functions explicitly\n\n compat/mingw.c | 12 +++++++++---\n compat/poll/poll.c | 2 +-\n compat/winansi.c | 10 ++++++----\n 3 files changed, 16 insertions(+), 8 deletions(-)\n\n\nbase-commit: aa25c82427ae70aebf3b8f970f2afd54e9a2a8c6\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-147%2Fdscho%2Fansi-unicode-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-147/dscho/ansi-unicode-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/147\n-- \ngitgitgadget\n"},{"id":"378147","messageId":"54fff4ef40d1dba2153de27413fb53e992228b5b.1561628237.git.gitgitgadget@gmail.com","threadId":"51395","inReplyTo":"pull.147.git.gitgitgadget@gmail.com","subject":"[PATCH 1/2] mingw: get pw_name in UTF-8 format","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-06-27T09:37:18Z","receivedAt":"2019-06-27T09:37:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nPreviously, we would have obtained the user name encoded in whatever the\ncurrent code page is.\n\nNote: the \"user name\" here does not denote the full name but instead the\nshort logon name.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/mingw.c | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 9b6d2400e1..8526876262 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1946,13 +1946,19 @@ struct passwd *getpwuid(int uid)\n \tstatic unsigned initialized;\n \tstatic char user_name[100];\n \tstatic struct passwd *p;\n+\twchar_t buf[100];\n \tDWORD len;\n \n \tif (initialized)\n \t\treturn p;\n \n-\tlen = sizeof(user_name);\n-\tif (!GetUserName(user_name, &len)) {\n+\tlen = sizeof(buf);\n+\tif (!GetUserNameW(buf, &len)) {\n+\t\tinitialized = 1;\n+\t\treturn NULL;\n+\t}\n+\n+\tif (xwcstoutf(user_name, buf, sizeof(user_name)) < 0) {\n \t\tinitialized = 1;\n \t\treturn NULL;\n \t}\n-- \ngitgitgadget\n\n"},{"id":"378148","messageId":"0c9d2aead5f729942968314679c5a1ca2e0370d0.1561628237.git.gitgitgadget@gmail.com","threadId":"51395","inReplyTo":"pull.147.git.gitgitgadget@gmail.com","subject":"[PATCH 2/2] mingw: use Unicode functions explicitly","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-06-27T09:37:19Z","receivedAt":"2019-06-27T09:37:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nMany Win32 API functions actually exist in two variants: one with\nthe `A` suffix that takes ANSI parameters (`char *` or `const char *`)\nand one with the `W` suffix that takes Unicode parameters (`wchar_t *`\nor `const wchar_t *`).\n\nThe ANSI variant assumes that the strings are encoded according to\nwhatever is the current locale. This is not what Git wants to use on\nWindows: we assume that `char *` variables point to strings encoded in\nUTF-8.\n\nThere is a pseudo UTF-8 locale on Windows, but it does not work\nas one might expect. In addition, if we overrode the user's locale, that\nwould modify the behavior of programs spawned by Git (such as editors,\ndifftools, etc), therefore we cannot use that pseudo locale.\n\nFurther, it is actually highly encouraged to use the Unicode versions\ninstead of the ANSI versions, so let's do precisely that.\n\nNote: when calling the Win32 API functions _without_ any suffix, it\ndepends whether the `UNICODE` constant is defined before the relevant\nheaders are #include'd. Without that constant, the ANSI variants are\nused. Let's be explicit and avoid that ambiguity.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/mingw.c | 2 +-\n compat/poll/poll.c | 2 +-\n compat/winansi.c | 10 ++++++----\n 3 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 8526876262..b8a62bf914 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1407,7 +1407,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen\n \tdo_unset_environment_variables();\n \n \t/* Determine whether or not we are associated to a console */\n-\tcons = CreateFile(\"CONOUT$\", GENERIC_WRITE,\n+\tcons = CreateFileW(L\"CONOUT$\", GENERIC_WRITE,\n \t\t\tFILE_SHARE_WRITE, NULL, OPEN_EXISTING,\n \t\t\tFILE_ATTRIBUTE_NORMAL, NULL);\n \tif (cons == INVALID_HANDLE_VALUE) {\ndiff --git a/compat/poll/poll.c b/compat/poll/poll.c\nindex 4459408c7d..8f24b80252 100644\n--- a/compat/poll/poll.c\n+++ b/compat/poll/poll.c\n@@ -150,7 +150,7 @@ win32_compute_revents (HANDLE h, int *p_sought)\n if (!once_only)\n \t{\n \t NtQueryInformationFile = (PNtQueryInformationFile)\n-\t GetProcAddress (GetModuleHandle (\"ntdll.dll\"),\n+\t GetProcAddress (GetModuleHandleW (L\"ntdll.dll\"),\n \t\t\t \"NtQueryInformationFile\");\n \t once_only = TRUE;\n \t}\ndiff --git a/compat/winansi.c b/compat/winansi.c\nindex f4f08237f9..cd947e048e 100644\n--- a/compat/winansi.c\n+++ b/compat/winansi.c\n@@ -599,7 +599,7 @@ int winansi_isatty(int fd)\n void winansi_init(void)\n {\n \tint con1, con2;\n-\tchar name[32];\n+\twchar_t name[32];\n \n \t/* check if either stdout or stderr is a console output screen buffer */\n \tcon1 = is_console(1);\n@@ -619,13 +619,15 @@ void winansi_init(void)\n \t}\n \n \t/* create a named pipe to communicate with the console thread */\n-\txsnprintf(name, sizeof(name), \"\\\\\\\\.\\\\pipe\\\\winansi%lu\", GetCurrentProcessId());\n-\thwrite = CreateNamedPipe(name, PIPE_ACCESS_OUTBOUND,\n+\tif (swprintf(name, ARRAY_SIZE(name) - 1, L\"\\\\\\\\.\\\\pipe\\\\winansi%lu\",\n+\t\t GetCurrentProcessId()) < 0)\n+\t\tdie(\"Could not initialize winansi pipe name\");\n+\thwrite = CreateNamedPipeW(name, PIPE_ACCESS_OUTBOUND,\n \t\tPIPE_TYPE_BYTE | PIPE_WAIT, 1, BUFFER_SIZE, 0, 0, NULL);\n \tif (hwrite == INVALID_HANDLE_VALUE)\n \t\tdie_lasterr(\"CreateNamedPipe failed\");\n \n-\thread = CreateFile(name, GENERIC_READ, 0, NULL, OPEN_EXISTING, 0, NULL);\n+\thread = CreateFileW(name, GENERIC_READ, 0, NULL, OPEN_EXISTING, 0, NULL);\n \tif (hread == INVALID_HANDLE_VALUE)\n \t\tdie_lasterr(\"CreateFile for named pipe failed\");\n \n-- \ngitgitgadget\n"},{"id":"378176","messageId":"CAPig+cTkzeyGjvftxmaA5Du2Dm4myzchJhrnHi=noJ7nLiLp-A@mail.gmail.com","threadId":"51395","inReplyTo":"0c9d2aead5f729942968314679c5a1ca2e0370d0.1561628237.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] mingw: use Unicode functions explicitly","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-06-27T17:26:45Z","receivedAt":"2019-06-27T17:27:01Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jun 27, 2019 at 5:37 AM Johannes Schindelin via GitGitGadget\n wrote:\n> Many Win32 API functions actually exist in two variants: one with\n> the `A` suffix that takes ANSI parameters (`char *` or `const char *`)\n> and one with the `W` suffix that takes Unicode parameters (`wchar_t *`\n> or `const wchar_t *`).\n>\n> The ANSI variant assumes that the strings are encoded according to\n> whatever is the current locale. This is not what Git wants to use on\n> Windows: we assume that `char *` variables point to strings encoded in\n> UTF-8.\n>\n> There is a pseudo UTF-8 locale on Windows, but it does not work\n> as one might expect.\n\nWhat does \"does not work as one might expect\" mean? The reader is left\nhanging, not knowing why or how the UTF-8 locale on Windows is\nundesirable.\n\n> In addition, if we overrode the user's locale, that\n> would modify the behavior of programs spawned by Git (such as editors,\n> difftools, etc), therefore we cannot use that pseudo locale.\n>\n> Further, it is actually highly encouraged to use the Unicode versions\n> instead of the ANSI versions, so let's do precisely that.\n>\n> Note: when calling the Win32 API functions _without_ any suffix, it\n> depends whether the `UNICODE` constant is defined before the relevant\n> headers are #include'd. Without that constant, the ANSI variants are\n> used. Let's be explicit and avoid that ambiguity.\n>\n> Signed-off-by: Johannes Schindelin \n"},{"id":"378188","messageId":"nycvar.QRO.7.76.6.1906272051240.44@tvgsbejvaqbjf.bet","threadId":"51395","inReplyTo":"CAPig+cTkzeyGjvftxmaA5Du2Dm4myzchJhrnHi=noJ7nLiLp-A@mail.gmail.com","subject":"Re: [PATCH 2/2] mingw: use Unicode functions explicitly","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-06-27T18:54:00Z","receivedAt":"2019-06-27T18:53:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Eric,\n\nOn Thu, 27 Jun 2019, Eric Sunshine wrote:\n\n> On Thu, Jun 27, 2019 at 5:37 AM Johannes Schindelin via GitGitGadget\n> wrote:\n> > Many Win32 API functions actually exist in two variants: one with\n> > the `A` suffix that takes ANSI parameters (`char *` or `const char *`)\n> > and one with the `W` suffix that takes Unicode parameters (`wchar_t *`\n> > or `const wchar_t *`).\n> >\n> > The ANSI variant assumes that the strings are encoded according to\n> > whatever is the current locale. This is not what Git wants to use on\n> > Windows: we assume that `char *` variables point to strings encoded in\n> > UTF-8.\n> >\n> > There is a pseudo UTF-8 locale on Windows, but it does not work\n> > as one might expect.\n>\n> What does \"does not work as one might expect\" mean? The reader is left\n> hanging, not knowing why or how the UTF-8 locale on Windows is\n> undesirable.\n\nShould I really bore the reader with half-details? At least one of those\nbehaviors was reported as an unbootable Windows 7, but some others could\nnot reproduce.\n\nI really did not want to write a novel here that is 1) largely irrelevant\nand 2) would take way too long to write because I forgot most of the\ndetails and would have to look them up again.\n\nHopefully we can do without the full story here, as even one report that\nit does not work as expected is enough to make this an unfeasible option\nfor us, and that's that as far as I am concerned.\n\nCiao,\nDscho\n\n>\n> > In addition, if we overrode the user's locale, that\n> > would modify the behavior of programs spawned by Git (such as editors,\n> > difftools, etc), therefore we cannot use that pseudo locale.\n> >\n> > Further, it is actually highly encouraged to use the Unicode versions\n> > instead of the ANSI versions, so let's do precisely that.\n> >\n> > Note: when calling the Win32 API functions _without_ any suffix, it\n> > depends whether the `UNICODE` constant is defined before the relevant\n> > headers are #include'd. Without that constant, the ANSI variants are\n> > used. Let's be explicit and avoid that ambiguity.\n> >\n> > Signed-off-by: Johannes Schindelin \n>\n"},{"id":"378605","messageId":"9bd9efde-8971-08f2-8417-1036286c9d1a@drbeat.li","threadId":"51395","inReplyTo":"54fff4ef40d1dba2153de27413fb53e992228b5b.1561628237.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] mingw: get pw_name in UTF-8 format","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2019-07-04T20:13:38Z","receivedAt":"2019-07-04T20:14:49Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"On 27.06.19 11:37, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> Previously, we would have obtained the user name encoded in whatever the\n> current code page is.\n> \n> Note: the \"user name\" here does not denote the full name but instead the\n> short logon name.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> compat/mingw.c | 10 ++++++++--\n> 1 file changed, 8 insertions(+), 2 deletions(-)\n> \n> diff --git a/compat/mingw.c b/compat/mingw.c\n> index 9b6d2400e1..8526876262 100644\n> --- a/compat/mingw.c\n> +++ b/compat/mingw.c\n> @@ -1946,13 +1946,19 @@ struct passwd *getpwuid(int uid)\n> \tstatic unsigned initialized;\n> \tstatic char user_name[100];\n> \tstatic struct passwd *p;\n> +\twchar_t buf[100];\n> \tDWORD len;\n> \n> \tif (initialized)\n> \t\treturn p;\n> \n> -\tlen = sizeof(user_name);\n> -\tif (!GetUserName(user_name, &len)) {\n> +\tlen = sizeof(buf);\n\nI think this should be \"len = sizeof(buf) / sizeof(buf[0])\".\n\nGetUserNameW() takes the number of characters, not bytes.\n\n> +\tif (!GetUserNameW(buf, &len)) {\n> +\t\tinitialized = 1;\n> +\t\treturn NULL;\n> +\t}\n> +\n> +\tif (xwcstoutf(user_name, buf, sizeof(user_name)) < 0) {\n> \t\tinitialized = 1;\n> \t\treturn NULL;\n> \t}\n> \n\nCheers,\nBeat\n"},{"id":"378607","messageId":"nycvar.QRO.7.76.6.1907042350510.44@tvgsbejvaqbjf.bet","threadId":"51395","inReplyTo":"9bd9efde-8971-08f2-8417-1036286c9d1a@drbeat.li","subject":"Re: [PATCH 1/2] mingw: get pw_name in UTF-8 format","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-07-04T21:52:54Z","receivedAt":"2019-07-04T21:52:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Beat,\n\nOn Thu, 4 Jul 2019, Beat Bolli wrote:\n\n> On 27.06.19 11:37, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin \n> >\n> > Previously, we would have obtained the user name encoded in whatever the\n> > current code page is.\n> >\n> > Note: the \"user name\" here does not denote the full name but instead the\n> > short logon name.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > compat/mingw.c | 10 ++++++++--\n> > 1 file changed, 8 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/compat/mingw.c b/compat/mingw.c\n> > index 9b6d2400e1..8526876262 100644\n> > --- a/compat/mingw.c\n> > +++ b/compat/mingw.c\n> > @@ -1946,13 +1946,19 @@ struct passwd *getpwuid(int uid)\n> > \tstatic unsigned initialized;\n> > \tstatic char user_name[100];\n> > \tstatic struct passwd *p;\n> > +\twchar_t buf[100];\n> > \tDWORD len;\n> >\n> > \tif (initialized)\n> > \t\treturn p;\n> >\n> > -\tlen = sizeof(user_name);\n> > -\tif (!GetUserName(user_name, &len)) {\n> > +\tlen = sizeof(buf);\n>\n> I think this should be \"len = sizeof(buf) / sizeof(buf[0])\".\n>\n> GetUserNameW() takes the number of characters, not bytes.\n\nGood catch.\n\nHow about I use `ARRAY_SIZE(buf)`? Since this is already in `next`, I will\nprepare a follow-up patch.\n\nCiao,\nDscho\n\n>\n> > +\tif (!GetUserNameW(buf, &len)) {\n> > +\t\tinitialized = 1;\n> > +\t\treturn NULL;\n> > +\t}\n> > +\n> > +\tif (xwcstoutf(user_name, buf, sizeof(user_name)) < 0) {\n> > \t\tinitialized = 1;\n> > \t\treturn NULL;\n> > \t}\n> >\n>\n> Cheers,\n> Beat\n>\n"},{"id":"378608","messageId":"1ab5bb1e-140a-cee1-0910-33e495cde11e@drbeat.li","threadId":"51395","inReplyTo":"nycvar.QRO.7.76.6.1907042350510.44@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/2] mingw: get pw_name in UTF-8 format","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2019-07-04T21:54:24Z","receivedAt":"2019-07-04T21:54:32Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"Hi Dscho\n\nOn 04.07.19 23:52, Johannes Schindelin wrote:\n> Hi Beat,\n> \n> On Thu, 4 Jul 2019, Beat Bolli wrote:\n> \n>> On 27.06.19 11:37, Johannes Schindelin via GitGitGadget wrote:\n>>> From: Johannes Schindelin \n>>>\n>>> Previously, we would have obtained the user name encoded in whatever the\n>>> current code page is.\n>>>\n>>> Note: the \"user name\" here does not denote the full name but instead the\n>>> short logon name.\n>>>\n>>> Signed-off-by: Johannes Schindelin \n>>> ---\n>>> compat/mingw.c | 10 ++++++++--\n>>> 1 file changed, 8 insertions(+), 2 deletions(-)\n>>>\n>>> diff --git a/compat/mingw.c b/compat/mingw.c\n>>> index 9b6d2400e1..8526876262 100644\n>>> --- a/compat/mingw.c\n>>> +++ b/compat/mingw.c\n>>> @@ -1946,13 +1946,19 @@ struct passwd *getpwuid(int uid)\n>>> \tstatic unsigned initialized;\n>>> \tstatic char user_name[100];\n>>> \tstatic struct passwd *p;\n>>> +\twchar_t buf[100];\n>>> \tDWORD len;\n>>>\n>>> \tif (initialized)\n>>> \t\treturn p;\n>>>\n>>> -\tlen = sizeof(user_name);\n>>> -\tif (!GetUserName(user_name, &len)) {\n>>> +\tlen = sizeof(buf);\n>>\n>> I think this should be \"len = sizeof(buf) / sizeof(buf[0])\".\n>>\n>> GetUserNameW() takes the number of characters, not bytes.\n> \n> Good catch.\n> \n> How about I use `ARRAY_SIZE(buf)`? Since this is already in `next`, I will\n> prepare a follow-up patch.\n\nOf course, ARRAY_SIZE() is even better. I didn't remember it existed.\n\nCheers,\nBeat\n\n> \n> Ciao,\n> Dscho\n> \n>>\n>>> +\tif (!GetUserNameW(buf, &len)) {\n>>> +\t\tinitialized = 1;\n>>> +\t\treturn NULL;\n>>> +\t}\n>>> +\n>>> +\tif (xwcstoutf(user_name, buf, sizeof(user_name)) < 0) {\n>>> \t\tinitialized = 1;\n>>> \t\treturn NULL;\n>>> \t}\n>>>\n>>\n>> Cheers,\n>> Beat\n>>\n\n"}]}