2013/5/15 Junio C Hamano <gits...@pobox.com>:
> Jiang Xin <worldhello....@gmail.com> writes:
>
>> 2013/5/15 Junio C Hamano <gits...@pobox.com>:
>>>> @@ -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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to