Sören Krecker*** BLURB HERE ***
soekkle (1): Replace SID with domain/username
compat/mingw.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-)
base-commit: e79552d19784ee7f4bbce278fe25f93fbda196fa
-- 2.39.2
threads / patch / 60668
patchReplace SID with domain/username on Windows
Subject: [PATCH 0/1] Replace SID with domain/username on Windows
Sören KreckerSören Krecker[PATCH 1/1] Replace SID with domain/username
From: soekkle <soekkle@freenet.de>
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 | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-)
Show changes to compat/mingw.c +23 −4
diff --git a/compat/mingw.c b/compat/mingw.c index 42053c1f65..7318f0e20e 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -2684,6 +2684,25 @@ static PSID get_current_user_sid(void) return result; } +BOOL user_sid_to_string(PSID sid, LPSTR* str) +{ + SID_NAME_USE peUse; + DWORD lenName = { 0 }, lenDomain = { 0 }; + LookupAccountSidA(NULL, sid, NULL, &lenName, NULL, + &lenDomain, &peUse); // returns only FALSE, because the string pointers are NULL + ALLOC_ARRAY((*str), (size_t)lenDomain + (size_t)lenName); // Alloc neded Space of the strings + BOOL retVal = LookupAccountSidA(NULL, sid, (*str) + lenDomain, &lenName, + *str, + &lenDomain, &peUse); + *(*str + lenDomain) = '/'; + if (retVal == FALSE) + { + free(*str); + *str = NULL; + } + return retVal; +} + static int acls_supported(const char *path) { size_t offset = offset_1st_component(path); @@ -2767,7 +2786,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_string(sid, &str1)) to_free1 = str1; else str1 = "(inconvertible)"; @@ -2776,7 +2795,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_string(current_user_sid, &str2)) to_free2 = str2; else str2 = "(inconvertible)"; @@ -2784,8 +2803,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); } }
-- 2.39.2
Eric SunshineRe: [PATCH 1/1] Replace SID with domain/username
On Thu, Dec 28, 2023 at 8:29 AM Sören Krecker <soekkle@freenet.de> wrote:
Show 7 quoted lines
> From: soekkle <soekkle@freenet.de> > > 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> > ---
I don't do Windows (anymore), thus I'm not qualified to comment on the substance of this patch, so I'll just make some general, hopefully helpful, observations.
Typo: "erromessage" should be "error message"
Your name in the "From:" header and Signed-off-by: should be the same.
Perhaps Widows folks can understand the purpose of this patch without further explanation, but for other readers, it's not clear what problem the patch is trying to solve. The commit message is a good place to explain _why_ this change is desirable.
> diff --git a/compat/mingw.c b/compat/mingw.c > @@ -2684,6 +2684,25 @@ static PSID get_current_user_sid(void) > +BOOL user_sid_to_string(PSID sid, LPSTR* str)
In this codebase, '*' sticks to the variable name, not the type, so:
BOOL user_sid_to_string(PSID sid, LPSTR *str)
> +{
> + SID_NAME_USE peUse;
> + DWORD lenName = { 0 }, lenDomain = { 0 };Looking through compat/mingw.c, it appears that (as with the rest of the project), variable names tend to use underscores rather than camel-case, so for consistency these might be better expressed as "pe_use" (whatever that means), "name_len", and "domain_len".
I was curious about the `{ 0 }` initializer. It seems we have a mix of both `{0}` and `{ 0 }` in the codebase, so what you have here is likely fine.
> + LookupAccountSidA(NULL, sid, NULL, &lenName, NULL, > + &lenDomain, &peUse); // returns only FALSE, because the string pointers are NULL
As with the rest of the project, compat/mingw.c still shuns "//" comments. Use /*...*/ comments instead.
> + ALLOC_ARRAY((*str), (size_t)lenDomain + (size_t)lenName); // Alloc neded Space of the strings
Type: "neded" -> "needed"
(and "Space" -> "space")
Show 8 quoted lines
> + BOOL retVal = LookupAccountSidA(NULL, sid, (*str) + lenDomain, &lenName,
> + *str,
> + &lenDomain, &peUse);
> + *(*str + lenDomain) = '/';
> + if (retVal == FALSE)
> + {
> + free(*str);
> + *str = NULL;The FREE_AND_NULL() macro from git-compat-util.h is a good companion to the ALLOC_ARRAY() macro used above, so freeing and nullifying could be done in one line:
FREE_AND_NULL(*str);
> + } > + return retVal; > +}
Perhaps a variable name such as `ok` would convey more to the reader than the generic `retVal`?