Re: [PATCH v2 1/1] Replace SID with domain/username

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 2, 2024, 17:33 UTC
Message-ID: <xmqqy1d7ej3d.fsf@gitster.g>
In-Reply-To: <xmqqplyjg10l.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 18 quoted lines

> 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.