Junio C Hamano <gitster@xxxxxxxxx> writes:
> Devin Lehmacher <lehmacdj@xxxxxxxxx> writes:
>
>> diff --git a/credential-cache.c b/credential-cache.c
>> index db1343b46..63236adc2 100644
>> --- a/credential-cache.c
>> +++ b/credential-cache.c
>> @@ -83,12 +83,18 @@ static void do_cache(const char *socket, const char *action, int timeout,
>> strbuf_release(&buf);
>> }
>>
>> +static int is_socket(char *path) {
>> + struct stat sb;
>> + int ret = lstat(path, &sb);
>> + return ret && S_IFSOCK(sb.st_mode);
>> +}
>> +
>> static char *get_socket_path(void) {
>> char *home_socket;
>>
>> home_socket = expand_user_path("~/.git-credential-cache/socket");
>> if (home_socket) {
>> - if (file_exists(home_socket))
>> + if (is_socket(home_socket))
>
> This should be done as part of 2/3, no? It does not make sense to
> add 2/3 and then immediately say "oops, the check in 2/3 is wrong,
> and let's update it like so".
Also I think you would want to use S_ISFIFO() and/or S_ISSOCK()
macros (I do not offhand recall which one credential cache daemon
uses), not the S_IFxxx constant.
Perhaps
1/3 - add xdg_cache similar to xdg_config
2/3 - add is_socket()
3/3 - use xdg_cache location for socket if traditional location
is not in use
would be a better logical ordering of the patches.
Having said that, I do not think ~/.git-credential-cache/socket is
the right thing to test. If the ~/.git-credential-cache directory
already exists, it is likely that the user has a set-up that works
well with "socket" inside it, and it is safer to keep using that
location.
On the other hand, if that directory does not exist, we know it is
safe to use whatever new location---after all, the lack of the
directory tells us that the user has never used the traditional
location successfully ;-)
So is_socket() may not even be needed, in which case this will be a
two-patch series:
1/2 - add xdg_cache similar to xdg_config
2/2 - use xdg_cache location if ~/.git-credential-cache/ is not there