Message ID | 20231229120319.3797-2-soekkle@freenet.de (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Replace SID with domain/username on Windows | expand |
Sören Krecker <soekkle@freenet.de> writes: > Replace SID with domain/username in erromessage, if owner of repository > and user are not equal on windows systems. "erromessage" -> "error messages" or something? This may not be a question raised by anybody who know Windows, but because I do not do Windows, it makes me wonder if this is losing information. Can two SID for the same user be active at the same time, which would cause user_sid_to_user_name() potentially yield the same string for two different SID? In any case, I am reasonably sure that Dscho will say yes or no to this patch (the above "makes me wonder" does not need to be resolved) and I can wait until then. Thanks. > Signed-off-by: Sören Krecker <soekkle@freenet.de> > --- > compat/mingw.c | 28 ++++++++++++++++++++++++---- > 1 file changed, 24 insertions(+), 4 deletions(-) > > diff --git a/compat/mingw.c b/compat/mingw.c > index 42053c1f65..05aeaaa9ad 100644 > --- a/compat/mingw.c > +++ b/compat/mingw.c > @@ -2684,6 +2684,26 @@ static PSID get_current_user_sid(void) > return result; > } > > +static BOOL user_sid_to_user_name(PSID sid, LPSTR *str) > +{ > + SID_NAME_USE pe_use; > + DWORD len_user = 0, len_domain = 0; > + BOOL translate_sid_to_user; > + > + /* returns only FALSE, because the string pointers are NULL*/ > + LookupAccountSidA(NULL, sid, NULL, &len_user, NULL, &len_domain, > + &pe_use); > + /*Alloc needed space of the strings*/ > + ALLOC_ARRAY((*str), (size_t)len_domain + (size_t)len_user); > + translate_sid_to_user = LookupAccountSidA(NULL, sid, (*str) + len_domain, &len_user, > + *str, &len_domain, &pe_use); > + *(*str + len_domain) = '/'; > + if (translate_sid_to_user == FALSE) { > + FREE_AND_NULL(*str); > + } > + return translate_sid_to_user; > +} > + > static int acls_supported(const char *path) > { > size_t offset = offset_1st_component(path); > @@ -2767,7 +2787,7 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report) > } else if (report) { > LPSTR str1, str2, to_free1 = NULL, to_free2 = NULL; > > - if (ConvertSidToStringSidA(sid, &str1)) > + if (user_sid_to_user_name(sid, &str1)) > to_free1 = str1; > else > str1 = "(inconvertible)"; > @@ -2776,7 +2796,7 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report) > str2 = "(none)"; > else if (!IsValidSid(current_user_sid)) > str2 = "(invalid)"; > - else if (ConvertSidToStringSidA(current_user_sid, &str2)) > + else if (user_sid_to_user_name(current_user_sid, &str2)) > to_free2 = str2; > else > str2 = "(inconvertible)"; > @@ -2784,8 +2804,8 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report) > "'%s' is owned by:\n" > "\t'%s'\nbut the current user is:\n" > "\t'%s'\n", path, str1, str2); > - LocalFree(to_free1); > - LocalFree(to_free2); > + free(to_free1); > + free(to_free2); > } > }
Junio C Hamano <gitster@pobox.com> writes: > Sören Krecker <soekkle@freenet.de> writes: > >> Replace SID with domain/username in erromessage, if owner of repository >> and user are not equal on windows systems. > > "erromessage" -> "error messages" or something? > > This may not be a question raised by anybody who know Windows, but > because I do not do Windows, it makes me wonder if this is losing > information. Can two SID for the same user be active at the same > time, which would cause user_sid_to_user_name() potentially yield > the same string for two different SID? > > In any case, I am reasonably sure that Dscho will say yes or no to > this patch (the above "makes me wonder" does not need to be > resolved) and I can wait until then. > > Thanks. Another thing I forgot to mention (but did wonder). The new helper function does allow LookupAccountSidA() to fail. Should it fall back to ConvertSidToStringSidA() that the original has been using? In any case, I do not think a failure to convert will result in an attempt to format ("%s", NULL) thanks to the existing code that uses the stringified SID, which is good.
diff --git a/compat/mingw.c b/compat/mingw.c index 42053c1f65..05aeaaa9ad 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -2684,6 +2684,26 @@ static PSID get_current_user_sid(void) return result; } +static BOOL user_sid_to_user_name(PSID sid, LPSTR *str) +{ + SID_NAME_USE pe_use; + DWORD len_user = 0, len_domain = 0; + BOOL translate_sid_to_user; + + /* returns only FALSE, because the string pointers are NULL*/ + LookupAccountSidA(NULL, sid, NULL, &len_user, NULL, &len_domain, + &pe_use); + /*Alloc needed space of the strings*/ + ALLOC_ARRAY((*str), (size_t)len_domain + (size_t)len_user); + translate_sid_to_user = LookupAccountSidA(NULL, sid, (*str) + len_domain, &len_user, + *str, &len_domain, &pe_use); + *(*str + len_domain) = '/'; + if (translate_sid_to_user == FALSE) { + FREE_AND_NULL(*str); + } + return translate_sid_to_user; +} + static int acls_supported(const char *path) { size_t offset = offset_1st_component(path); @@ -2767,7 +2787,7 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report) } else if (report) { LPSTR str1, str2, to_free1 = NULL, to_free2 = NULL; - if (ConvertSidToStringSidA(sid, &str1)) + if (user_sid_to_user_name(sid, &str1)) to_free1 = str1; else str1 = "(inconvertible)"; @@ -2776,7 +2796,7 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report) str2 = "(none)"; else if (!IsValidSid(current_user_sid)) str2 = "(invalid)"; - else if (ConvertSidToStringSidA(current_user_sid, &str2)) + else if (user_sid_to_user_name(current_user_sid, &str2)) to_free2 = str2; else str2 = "(inconvertible)"; @@ -2784,8 +2804,8 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report) "'%s' is owned by:\n" "\t'%s'\nbut the current user is:\n" "\t'%s'\n", path, str1, str2); - LocalFree(to_free1); - LocalFree(to_free2); + free(to_free1); + free(to_free2); } }
Replace SID with domain/username in erromessage, if owner of repository and user are not equal on windows systems. Signed-off-by: Sören Krecker <soekkle@freenet.de> --- compat/mingw.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-)