Re: [PATCH v7 2/6] fsmonitor: relocate socket file if .git directory is remote

> On 9/16/22 4:11 PM, Junio C Hamano wrote:
>> "Eric DeCosta via GitGitGadget" <gitgitgadget@gmail.com> writes:
>> 
>>> +const char *fsmonitor_ipc__get_path(struct repository *r)
>>> +{
>>> +	static const char *ipc_path;
>>> +	SHA_CTX sha1ctx;
>>> +	char *sock_dir;
>>> +	struct strbuf ipc_file = STRBUF_INIT;
>>> +	unsigned char hash[SHA_DIGEST_LENGTH];
>>> +
>>> +	if (ipc_path)
>>> +		return ipc_path;
>>> +
>>> +	if (!r)
>>> +		r = the_repository;
>> I'd prefer not to see this "NULL means the_repository".  It would be
>> a different story if the caller does not necessarily have a ready
>> access to the_repository, but it is a global, so the caller can pass
>> the_repository and be more explicit.  Giving two ways to the caller
>> to express same thing is not a good idea.
>> Thanks.
>> 
>
> To be fair, I added several "if (!r) r = the_repository;" statements
> to the original public FSMonitor routines.  There were obscure cases
> where tests would sometimes randomly fail because "r" wasn't completely
> passed down via some hard to isolate call stack.  Offlist, AEvar told me
> that he managed to isolate it and has a fix.
>
> So eventually, we'll be able to get rid of all of these direct
> references to "the_repository" and properly assume that "r" is
> always passed down.
>
> But for now, I think we should let this stay for safety.