Provide a useful default user ident on Windows
cover Provide a useful default user ident on Windows
On Linux, we use the gecos information to come up with a sensible user name/email. On Windows, there is no gecos. But there is something comparable, and with these patches, we use it.
This has been carried in Git for Windows for three years, and is considered mature.
Johannes Schindelin (3):
getpwuid(mingw): initialize the structure only once getpwuid(mingw): provide a better default for the user name
mingw: use domain information for default email
compat/mingw.c | 60 +++++++++++++++++++++++++++++++++++++++++------ compat/mingw.h | 2 ++ git-compat-util.h | 4 ++++ ident.c | 3 +++ 4 files changed, 62 insertions(+), 7 deletions(-)
base-commit: 5a0cc8aca797dbd7d2be3b67458ff880ed45cddf
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-45%2Fdscho%2Fdefault-ident-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-45/dscho/default-ident-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/45
Signature · 1 line
gitgitgadget
PATCH 1/3 getpwuid(mingw): initialize the structure only once
From: Johannes Schindelin <[email protected]>
--- compat/mingw.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-)
compat/mingw.c +18 -7
@@ -1800,16 +1800,27 @@ int mingw_getpagesize(void)struct passwd *getpwuid (int uid) { +static unsigned initialized;static char user_name[100 ]; -static struct passwd p; +static struct passwd *p; +DWORD len; -DWORD len =sizeof (user_name); -if (!GetUserName (user_name, &len)) +if (initialized) +return p; + + len =sizeof (user_name); +if (!GetUserName (user_name, &len)) { + initialized =1 ;return NULL ; - p.pw_name = user_name; - p.pw_gecos ="unknown" ; - p.pw_dir =NULL ; -return &p; + } + + p =xmalloc (sizeof (*p)); + p->pw_name = user_name; + p->pw_gecos ="unknown" ; + p->pw_dir =NULL ; + + initialized =1 ; +return p; }static HANDLE timer_event;
Signature · 1 line
gitgitgadget
PATCH 2/3 getpwuid(mingw): provide a better default for the user name
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
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.
--- compat/mingw.c | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-)
compat/mingw.c +31 -1
@@ -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, &len)) { +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 ;
Signature · 1 line
gitgitgadget
PATCH 3/3 mingw: use domain information for default email
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
When a user is registered in a Windows domain, it is really easy to obtain the email address. So let's do that.
Suggested by Lutz Roeder.
--- compat/mingw.c | 5 +++++ compat/mingw.h | 2 ++ git-compat-util.h | 4 ++++ ident.c | 3 +++ 4 files changed, 14 insertions(+)
compat/mingw.c +5 -0
@@ -1826,6 +1826,11 @@ static char *get_extended_user_info(enum EXTENDED_NAME_FORMAT type)return NULL ; } +char *mingw_query_user_email (void ) +{ +return get_extended_user_info (NameUserPrincipal); +} +struct passwd *getpwuid (int uid) {static unsigned initialized;
compat/mingw.h +2 -0
@@ -424,6 +424,8 @@ static inline void convert_slashes(char *path)int mingw_offset_1st_component (const char *path);#define offset_1st_component mingw_offset_1st_component#define PATH_SEP ';' +extern char *mingw_query_user_email (void ); +#define query_user_email mingw_query_user_email#if !defined(__MINGW64_VERSION_MAJOR) && (!defined(_MSC_VER) || _MSC_VER <1800 )#define PRIuMAX "I64u"#define PRId64 "I64d"
git-compat-util.h +4 -0
@@ -382,6 +382,10 @@ static inline char *git_find_last_dir_sep(const char *path)#define find_last_dir_sep git_find_last_dir_sep#endif +#ifndef query_user_email +#define query_user_email () NULL +#endif +#if defined(__HP_cc) && (__HP_cc >=61000 )#define NORETURN __attribute__((noreturn))#define NORETURN_PTR
ident.c +3 -0
@@ -168,6 +168,9 @@ const char *ident_default_email(void)strbuf_addstr (&git_default_email, email); committer_ident_explicitly_given |=IDENT_MAIL_GIVEN ; author_ident_explicitly_given |=IDENT_MAIL_GIVEN ; + }else if ((email =query_user_email ()) && email[0 ]) { +strbuf_addstr (&git_default_email, email); +free ((char *)email); }else copy_email (xgetpwuid_self (&default_email_is_bogus), &git_default_email, &default_email_is_bogus);
Signature · 1 line
gitgitgadget
Re: [PATCH 1/3] getpwuid(mingw): initialize the structure only once
In reply to Johannes Schindelin via GitGitGadgetQuoted 9 lines · Johannes Schindelin via GitGitGadget
Signed-off-by Johannes Schindelin <[email protected]>---
compat/mingw.c +1
-1@@ -1800,16 +1800,27 @@ int mingw_getpagesize(void)struct passwd *getpwuid (int uid) { +static unsigned initialized;static char user_name[100 ]; -static struct passwd p;
compat/mingw.c int mingw_getpagesize(void) · line 1813
Quoted 10 lines
compat/mingw.c +8
-0+static struct passwd *p; +if (initialized) +return p; + + len =sizeof (user_name); +if (!GetUserName (user_name, &len)) { + initialized =1 ;return NULL ; + }
If GetUserName() fails, that failure is recorded (as "initialized=1" and 'p' is still NULL), so subsequent invocations just return NULL without doing any more work. Makes sense.
Quoted 8 lines
compat/mingw.c +7
-0+ p =xmalloc (sizeof (*p)); + p->pw_name = user_name; + p->pw_gecos ="unknown" ; + p->pw_dir =NULL ; + + initialized =1 ; +return p; }
Re: [PATCH 2/3] getpwuid(mingw): provide a better default for the user name
In reply to Johannes Schindelin via GitGitGadgetcompat/mingw.c int mingw_getpagesize(void) · line 1803
Quoted 13 lines · Johannes Schindelin via GitGitGadget
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 <[email protected]>---
compat/mingw.c +5
-0@@ -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.
compat/mingw.c int mingw_getpagesize(void) · line 1824
Quoted 7 lines
compat/mingw.c +7
-0+ len =ARRAY_SIZE (wbuffer); +if (GetUserNameExW (type, wbuffer, &len)) { +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.
Quoted 2 lines
compat/mingw.c +2
-0+return NULL ; +}
Re: [PATCH 3/3] mingw: use domain information for default email
In reply to Johannes Schindelin via GitGitGadgetQuoted 13 lines · Johannes Schindelin via GitGitGadget
When a user is registered in a Windows domain, it is really easy to obtain the email address. So let's do that.
[...]Signed-off-by Johannes Schindelin <[email protected]>---
compat/mingw.c +4
-0@@ -1826,6 +1826,11 @@ static char *get_extended_user_info(enum EXTENDED_NAME_FORMAT type) +char *mingw_query_user_email (void ) +{ +return get_extended_user_info (NameUserPrincipal); +}
ident.c const char *ident_default_email(void) · line 170
Quoted 3 lines
ident.c +3
-0@@ -168,6 +168,9 @@ const char *ident_default_email(void) + }else if ((email =query_user_email ()) && email[0 ]) { +strbuf_addstr (&git_default_email, email); +free ((char *)email);
If query_user_email(), which calls get_extended_user_info(), returns NULL, then we do nothing (and don't worry about freeing the result). However, if query_user_email() returns a zero-length allocated string, then this conditional will leak that string (due to the email[0] check). From patch 2/3, we see in get_extended_user_info():
+static char *get_extended_user_info(enum EXTENDED_NAME_FORMAT type) +{
+ if (GetUserNameExW(type, wbuffer, &len)) {
+ char *converted = xmalloc((len *= 3));
+ if (xwcstoutf(converted, wbuffer, len) >= 0)
+ return converted;
that it may possibly return a zero-length string (due to the ">=0"). Do we care about this potential leak (or is GetUserNameExW() guaranteed never to return such a string)?
Re: [PATCH 3/3] mingw: use domain information for default email
In reply to Johannes Schindelin via GitGitGadget"Johannes Schindelin via GitGitGadget" <[email protected]>
compat/mingw.c static char *get_extended_user_info(enum EXTENDED_NAME_FORMAT type)
Quoted 10 lines · writes:
compat/mingw.c +8
-0+char *mingw_query_user_email (void ) +{ +return get_extended_user_info (NameUserPrincipal); +} + +#ifndef query_user_email +#define query_user_email () NULL +#endif
The three patches look sensible to me; will queue.
You may already have audited our use of "struct passwd", "getpwnam()" and "getpwuid()"--I haven't. I think we use these only to learn user's email (to be used as the default ident) and home directory (to expand "git config --type=path"). If that is really the case, it may be a worthwhile clean-up to introduce our own API that offers these two exact functions, have the per-platform implementation of the API in compat/, and get rid of "struct passwd" and calls to getpw*() functions out of the core Git proper, to wean ourselves away from depending on POSIX too much.
But of course that is a separate topic.
Re: [PATCH 3/3] mingw: use domain information for default email
In reply to Eric SunshineHi Eric,
Quoted 32 lines · Eric Sunshine
Quoted 13 lines · Johannes Schindelin via GitGitGadget
When a user is registered in a Windows domain, it is really easy to obtain the email address. So let's do that.
[...]Signed-off-by Johannes Schindelin <[email protected]>---
compat/mingw.c +4
-0@@ -1826,6 +1826,11 @@ static char *get_extended_user_info(enum EXTENDED_NAME_FORMAT type) +char *mingw_query_user_email (void ) +{ +return get_extended_user_info (NameUserPrincipal); +}ident.c const char *ident_default_email(void) · line 170
Quoted 3 lines
ident.c +3
-0@@ -168,6 +168,9 @@ const char *ident_default_email(void) + }else if ((email =query_user_email ()) && email[0 ]) { +strbuf_addstr (&git_default_email, email); +free ((char *)email);If
query_user_email(), which callsget_extended_user_info(), returns NULL, then we do nothing (and don't worry about freeing the result). However, ifquery_user_email()returns a zero-length allocated string, then this conditional will leak that string (due to the email[0] check). From patch 2/3, we see inget_extended_user_info():(quoted diff) +6
-0+static char *get_extended_user_info(enum EXTENDED_NAME_FORMAT type) +{ + if (GetUserNameExW(type, wbuffer, &len)) { + char *converted = xmalloc((len *= 3)); + if (xwcstoutf(converted, wbuffer, len) >= 0) + return converted;
Quoted 3 lines
that it may possibly return a zero-length string (due to the ">=0"). Do we care about this potential leak (or is
GetUserNameExW()guaranteed never to return such a string)?
Quite honestly, I think that this is so rare an instance (if it *can* happen at all) that I simply don't care ;-)
Ciao,
Dscho
Re: [PATCH 2/3] getpwuid(mingw): provide a better default for the user name
In reply to Eric SunshineHi Eric,
Quoted 17 lines · Eric Sunshine
compat/mingw.c int mingw_getpagesize(void) · line 1803
Quoted 13 lines · Johannes Schindelin via GitGitGadget
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 <[email protected]>---
compat/mingw.c +5
-0@@ -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.
Quoted 3 lines
compat/mingw.c int mingw_getpagesize(void) · line 1820
Quoted 3 lines
compat/mingw.c +3
-0+ len =ARRAY_SIZE (wbuffer); +if (GetUserNameExW (type, wbuffer, &len)) { +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
Quoted 12 lines
compat/mingw.c int mingw_getpagesize(void) · line 1824
Quoted 4 lines
compat/mingw.c +4
-0+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.Quoted 2 lines
compat/mingw.c +2
-0+return NULL ; +}
Re: [PATCH 2/3] getpwuid(mingw): provide a better default for the user name
Quoted 10 lines · Johannes Schindelin
Quoted 5 lines · Eric Sunshine
compat/mingw.c int mingw_getpagesize(void) · line 1820
Quoted 3 lines · Johannes Schindelin via GitGitGadget
compat/mingw.c +3
-0+ len =ARRAY_SIZE (wbuffer); +if (GetUserNameExW (type, wbuffer, &len)) { +char *converted =xmalloc ((len *=3 ));Oh wow. I *just* realized this. It is an off-by-one: the
xmalloc()call needs to receivelen + 1to 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,
Quoted 9 lines · Eric Sunshine
Quoted 7 lines · Johannes Schindelin
Quoted 5 lines · Eric Sunshine
Quoted 3 lines · Johannes Schindelin via GitGitGadget
compat/mingw.c +3
-0+ len =ARRAY_SIZE (wbuffer); +if (GetUserNameExW (type, wbuffer, &len)) { +char *converted =xmalloc ((len *=3 ));
Quoted 6 lines
Quoted 3 lines
Oh wow. I *just* realized this. It is an off-by-one: the
xmalloc()call needs to receivelen + 1to 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