Devin Lehmacher <lehmacdj@xxxxxxxxx> writes:
> +int directory_exists(const char *path)
> +{
> + struct stat sb;
> + int ret = lstat(path, &sb);
> + return ret == 0 && S_ISDIR(sb.st_mode);
> +}
I am not a great fan of using file_exists() [*1*] on anything other
than paths in the working tree (i.e. in preparation for checking
things out of or into the index), as paths inside .git/ and paths
outside have different requirements. One of the difference is if it
makes sense to use stat(2) or lstat(2) for a check like this
function does. For working tree entities, which file_exists() and
friends are designed for, we absolutely should use lstat(2). The
"RelNotes" symbolic link at the top level of the project must be
known as a symbolic link and a question "is this a directory?" on it
must be answered "no", for example.
But there is no fundamental reason ~/.git-credential-cache (or
anything outside the working tree) must be S_ISDIR(). If the user
wanted to have the real directory elsewhere and point at it with
a symbolic link ~/.git-credential-cache, we should allow it.
If we need a helper function to see if a path in the working tree is
a directory, adding this new helper function to dir.c (which is
about the paths in the working tree) and use lstat(2) in it is the
right thing to do. But I do not think it should be used to check if
~/.git-credential-cache directory, which is not a filesystem entity
in a working tree, exists.
IOW, I do not think this patch helps the topic of this series. Drop
this patch from the series and have a similar code (but use stat(2)
instead of lstat(2)) directly inside get_socket_path()'s in the next
patch, perhaps?
[Footnote]
*1* ... and friends, like safe_create_leading_directories().