"Glen Choo via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> iter = dir_iterator_begin(src->buf, DIR_ITERATOR_PEDANTIC);
>
> - if (!iter)
> + if (!iter) {
> + struct stat st;
> + if (lstat(src->buf, &st) >= 0 && S_ISLNK(st.st_mode))
> + die(_("'%s' is a symlink, refusing to clone with --local"),
> + src->buf);
If you want to do lstat(2) yourself, the canonical way to check its
success is to see the returned value is 0, not "not negative", but
let's first see how dir_iterator_begin() can fail. I suspect it may
not necessary to do another lstat(2). The function returns NULL:
* if lstat() fails; errno is left from the failed lstat() in this case.
* if lstat() succeeds, but the path is *not* a directory; errno is
explicitly set to ENOTDIR. Unfortunately, if lstat(2) failed
with ENOTDIR (e.g. dir_iterator_begin() gets called with a path
whose leading component is not a directory), the caller will also
see ENOTDIR, but the distinction may not matter in practice. I
haven't thought things through.
Assuming that the distinction does not matter, then,
if (!iter) {
if (errno == ENOTDIR)
die(_("'%s' is not a directory, refusing to clone with --local"),
src->buf);
else
die_errno(_("failed to stat '%s'"), src->buf);
}
may be sufficient. But because this is an error codepath, it is not
worth optimizing what happens there, and an extra lstat(2) is not
too bad, if the code gains extra clarity.
