From: Farid Zakaria Date: Wed, 04 Nov 2026 18:25:46 GMT Subject: Re: [PATCH 1/2] credential-cache--daemon: allow a concurrently created socket dir Message-ID: In-Reply-To: On Tue Nov 3, 2026 at 12:01 AM PST, Patrick Steinhardt wrote: > On Sun, Nov 01, 2026 at 03:29:25PM -0800, Farid Zakaria wrote: >> diff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c >> index 65cc619bec..0dfa11e2ec 100644 >> --- a/builtin/credential-cache--daemon.c >> +++ b/builtin/credential-cache--daemon.c >> @@ -273,8 +273,18 @@ static void init_socket_directory(const char *path) >> */ >> if (safe_create_leading_directories_const(the_repository, dir) < 0) >> die_errno("unable to create directories for '%s'", dir); > > It's quite curious that we use `safe_create_leading_directories()` here. > As documented in "path.h", the `safe_...()` functions should only ever > be used for creating directories in the ".git" directory itself. In > particular, applying "core.sharedRepostiory" to the leading directories > of our credential daemon is never going to result in anything good at > all. > > So we should call `safe_create_leadingdirectories_no_share_const()` > instead, which does not apply "core.sharedRepository". I know that's not > introduced by your patch, but would you mind maybe fixing that in a > preparatory commit? > Sure, I'll add that as the first patch in v2. >> - if (mkdir(dir, 0700) < 0) >> - die_errno("unable to mkdir '%s'", dir); >> + if (mkdir(dir, 0700) < 0) { >> + if (errno != EEXIST) >> + die_errno("unable to mkdir '%s'", dir); > > Makes sense. > >> + /* >> + * Another daemon created it first; check it like one >> + * that already existed. >> + */ >> + if (stat(dir, &st)) >> + die_errno("unable to stat '%s'", dir); >> + if (st.st_mode & 077) >> + die(_(permissions_advice), dir); >> + } > > A bit of a shame that we duplicate this logic, but I don't see an > obvious way for how to avoid that. > > Patrick