Re: [PATCH 2/3] getpwuid(mingw): provide a better default for the user name
Hi Eric,
On Tue, 16 Oct 2018, Eric Sunshine wrote:
> On Tue, Oct 16, 2018 at 8:38 AM Johannes Schindelin
> wrote:
> > On Mon, 15 Oct 2018, Eric Sunshine wrote:
> > > On Mon, Oct 15, 2018 at 5:47 AM Johannes Schindelin via GitGitGadget
> > > wrote:
> > > > + len = ARRAY_SIZE(wbuffer);
> > > > + if (GetUserNameExW(type, wbuffer, )) {
> > > > + char *converted = xmalloc((len *= 3));
> >
> > Oh wow. I *just* realized this. It is an off-by-one: the `xmalloc()` call
> > needs to receive `len + 1` to accommodate for the trailing NUL. Will fix,
> > too.
>
> Or, xmallocz() (note the "z") which gives you overflow-checking of the
> +1 for free.
Very good point! Thanks for the suggestion,
Dscho
Re: [PATCH 2/3] getpwuid(mingw): provide a better default for the user name
On Tue, Oct 16, 2018 at 8:38 AM Johannes Schindelin
wrote:
> On Mon, 15 Oct 2018, Eric Sunshine wrote:
> > On Mon, Oct 15, 2018 at 5:47 AM Johannes Schindelin via GitGitGadget
> > wrote:
> > > + len = ARRAY_SIZE(wbuffer);
> > > + if (GetUserNameExW(type, wbuffer, )) {
> > > + char *converted = xmalloc((len *= 3));
>
> Oh wow. I *just* realized this. It is an off-by-one: the `xmalloc()` call
> needs to receive `len + 1` to accommodate for the trailing NUL. Will fix,
> too.
Or, xmallocz() (note the "z") which gives you overflow-checking of the
+1 for free.
Re: [PATCH 2/3] getpwuid(mingw): provide a better default for the user name
Hi Eric,
On Mon, 15 Oct 2018, Eric Sunshine wrote:
> On Mon, Oct 15, 2018 at 5:47 AM Johannes Schindelin via GitGitGadget
> wrote:
> > We do have the excellent GetUserInfoEx() function to obtain more
> > detailed information of the current user (if the user is part of a
> > Windows domain); Let's use it.
> > [...]
> > Signed-off-by: Johannes Schindelin
> > ---
> > diff --git a/compat/mingw.c b/compat/mingw.c
> > @@ -1798,6 +1799,33 @@ int mingw_getpagesize(void)
> > +static char *get_extended_user_info(enum EXTENDED_NAME_FORMAT type)
> > +{
> > + DECLARE_PROC_ADDR(secur32.dll, BOOL, GetUserNameExW,
> > + enum EXTENDED_NAME_FORMAT, LPCWSTR, PULONG);
> > + static wchar_t wbuffer[1024];
>
> Does this need to be static? It's not being returned to the caller.
It does not. Fixed.
> > + len = ARRAY_SIZE(wbuffer);
> > + if (GetUserNameExW(type, wbuffer, )) {
> > + char *converted = xmalloc((len *= 3));
Oh wow. I *just* realized this. It is an off-by-one: the `xmalloc()` call
needs to receive `len + 1` to accommodate for the trailing NUL. Will fix,
too.
Ciao,
Dscho
> > + if (xwcstoutf(converted, wbuffer, len) >= 0)
> > + return converted;
> > + free(converted);
> > + }
>
> If xwcstoutf() fails, 'converted' is freed; otherwise, the allocated
> 'converted' is stored in the caller's statically held 'passwd' struct.
> Okay.
>
> > + return NULL;
> > +}
>
Re: [PATCH 2/3] getpwuid(mingw): provide a better default for the user name
On Mon, Oct 15, 2018 at 5:47 AM Johannes Schindelin via GitGitGadget
wrote:
> We do have the excellent GetUserInfoEx() function to obtain more
> detailed information of the current user (if the user is part of a
> Windows domain); Let's use it.
> [...]
> Signed-off-by: Johannes Schindelin
> ---
> diff --git a/compat/mingw.c b/compat/mingw.c
> @@ -1798,6 +1799,33 @@ int mingw_getpagesize(void)
> +static char *get_extended_user_info(enum EXTENDED_NAME_FORMAT type)
> +{
> + DECLARE_PROC_ADDR(secur32.dll, BOOL, GetUserNameExW,
> + enum EXTENDED_NAME_FORMAT, LPCWSTR, PULONG);
> + static wchar_t wbuffer[1024];
Does this need to be static? It's not being returned to the caller.
> + len = ARRAY_SIZE(wbuffer);
> + if (GetUserNameExW(type, wbuffer, )) {
> + char *converted = xmalloc((len *= 3));
> + if (xwcstoutf(converted, wbuffer, len) >= 0)
> + return converted;
> + free(converted);
> + }
If xwcstoutf() fails, 'converted' is freed; otherwise, the allocated
'converted' is stored in the caller's statically held 'passwd' struct.
Okay.
> + return NULL;
> +}
[PATCH 2/3] getpwuid(mingw): provide a better default for the user name
From: Johannes Schindelin
We do have the excellent GetUserInfoEx() function to obtain more
detailed information of the current user (if the user is part of a
Windows domain); Let's use it.
Suggested by Lutz Roeder.
To avoid the cost of loading Secur32.dll (even lazily, loading DLLs
takes a non-neglibile amount of time), we use the established technique
to load DLLs only when, and if, needed.
Signed-off-by: Johannes Schindelin
---
compat/mingw.c | 32 +++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 597781b370..623ff5daf5 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -5,6 +5,7 @@
#include "../strbuf.h"
#include "../run-command.h"
#include "../cache.h"
+#include "win32/lazyload.h"
#define HCAST(type, handle) ((type)(intptr_t)handle)
@@ -1798,6 +1799,33 @@ int mingw_getpagesize(void)
return si.dwAllocationGranularity;
}
+/* See https://msdn.microsoft.com/en-us/library/windows/desktop/ms724435.aspx
*/
+enum EXTENDED_NAME_FORMAT {
+ NameDisplay = 3,
+ NameUserPrincipal = 8
+};
+
+static char *get_extended_user_info(enum EXTENDED_NAME_FORMAT type)
+{
+ DECLARE_PROC_ADDR(secur32.dll, BOOL, GetUserNameExW,
+ enum EXTENDED_NAME_FORMAT, LPCWSTR, PULONG);
+ static wchar_t wbuffer[1024];
+ DWORD len;
+
+ if (!INIT_PROC_ADDR(GetUserNameExW))
+ return NULL;
+
+ len = ARRAY_SIZE(wbuffer);
+ if (GetUserNameExW(type, wbuffer, )) {
+ char *converted = xmalloc((len *= 3));
+ if (xwcstoutf(converted, wbuffer, len) >= 0)
+ return converted;
+ free(converted);
+ }
+
+ return NULL;
+}
+
struct passwd *getpwuid(int uid)
{
static unsigned initialized;
@@ -1816,7 +1844,9 @@ struct passwd *getpwuid(int uid)
p = xmalloc(sizeof(*p));
p->pw_name = user_name;
- p->pw_gecos = "unknown";
+ p->pw_gecos = get_extended_user_info(NameDisplay);
+ if (!p->pw_gecos)
+ p->pw_gecos = "unknown";
p->pw_dir = NULL;
initialized = 1;
--
gitgitgadget
