2013/5/15 Junio C Hamano <gitster@xxxxxxxxx>:
> Jiang Xin <worldhello.net@xxxxxxxxx> writes:
>
>> 2013/5/15 Junio C Hamano <gitster@xxxxxxxxx>:
>>>> @@ -242,11 +287,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>>>> continue; /* Yup, this one exists unmerged */
>>>> }
>>>>
>>>> - /*
>>>> - * we might have removed this as part of earlier
>>>> - * recursive directory removal, so lstat() here could
>>>> - * fail with ENOENT.
>>>> - */
>>>> if (lstat(ent->name, &st))
>>>> continue;
>>>
>>> I am guessing that the reason why you removed the comment is because
>>> during this phase there is no way we "might have removed". But if
>>> that is the case, does it still make sense to run lstat() and ignore
>>> errors from the call?
>>>
>>
>> Run lstat() here is necessary, because we need to check whether
>> ent->name points to a file or a directory. If ent points to a directory,
>> only add to del_list when user provides '-x' option to git-clean.
>
> Sorry, but that was not the question; we can see st is used
> immediately below so somebody needs to fill it.
>
> I was pointing out that the "lstat() is expected to fail with ENOENT
> but it is not an error worth reporting" justification the original
> code had to silently ignore an error, because you no longer remove
> anything immediately in this part of the code. Is "if () continue"
> still valid thing to do here, not "if () die()"?
I'm clear, it could be:
if (lstat(ent->name, &st))
die_errno("Cannot lstat '%s'", ent->name);
--
Jiang Xin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html