Subject: Re: [PATCH v6 3/6] fsmonitor: relocate socket file if .git directory is remote

From: Junio C Hamano <[email-masked]>
Date: Tue, 13 Sep 2022 17:48:09 -0700

"Eric DeCosta via GitGitGadget" <[email-masked]> writes:

> +const char *fsmonitor_ipc__get_path(void)
> +{

Looks like a bit klunky API.  I would have expected it to take at
least the path to the worktree or a pointer to struct repository.

> +	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;
> +
> +	ipc_path = fsmonitor_ipc__get_default_path();
> +
> +	/* By default the socket file is created in the .git directory */
> +	if (fsmonitor__is_fs_remote(ipc_path) < 1)
> +		return ipc_path;
> +
> +	SHA1_Init(&sha1ctx);
> +	SHA1_Update(&sha1ctx, the_repository->worktree, strlen(the_repository->worktree));
> +	SHA1_Final(hash, &sha1ctx);

I would not worry about SHA-1 hash collision for this use case, but
would worry more about .worktree possible being unique.

Can the .worktree string be aliased for the same directory in some
way (e.g. depending on the initialization sequence, can it be a full
pathname, a relative pathname, or can a pathname that looks like a
full-pathname have symbolic link component in it?) that ends up
creating two or more socket for the same worktree?

> +	repo_config_get_string(the_repository, "fsmonitor.socketdir", &sock_dir);
> +
> +	/* Create the socket file in either socketDir or $HOME */
> +	if (sock_dir && *sock_dir)
> +		strbuf_addf(&ipc_file, "%s/.git-fsmonitor-%s",
> +					sock_dir, hash_to_hex(hash));
> +	else
> +		strbuf_addf(&ipc_file, "~/.git-fsmonitor-%s", hash_to_hex(hash));
> +
> +	ipc_path = interpolate_path(ipc_file.buf, 1);
> +	if (!ipc_path)
> +		die(_("Invalid path: %s"), ipc_file.buf);
> +
> +	strbuf_release(&ipc_file);
> +	return ipc_path;
> +}