On Mon, May 2, 2016 at 1:40 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Sun, May 1, 2016 at 7:14 AM, Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote:
>> "err" is deleted because it just causes confusion when "errno" is also
>> used directly in process_lstat_error().
>
> Despite the function name which may imply that it is consulting errno,
> this change makes me feel slightly uncomfortable since it increases
> coupling. Whereas consulting of errno had been explicit, with this
> change, it becomes implicit, and someone changing the code needs to be
> extra careful about ensuring that errno does not get clobbered.
>
> Not a big objection, but a nagging doubt...
We could s/errno/err/ in this function and not convert to
error_errno(). Or we could delete this function and move the code back
to its only call site in process_path()?
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
>> ---
>> diff --git a/builtin/update-index.c b/builtin/update-index.c
>> @@ -251,11 +251,11 @@ static int remove_one_path(const char *path)
>> -static int process_lstat_error(const char *path, int err)
>> +static int process_lstat_error(const char *path)
>> {
>> - if (err == ENOENT || err == ENOTDIR)
>> + if (errno == ENOENT || errno == ENOTDIR)
>> return remove_one_path(path);
>> - return error("lstat(\"%s\"): %s", path, strerror(errno));
>> + return error_errno("lstat(\"%s\")", path);
>> }
>>
>> static int add_one_path(const struct cache_entry *old, const char *path, int len, struct stat *st)
>> @@ -382,7 +382,7 @@ static int process_path(const char *path)
>> * what to do about the pathname!
>> */
>> if (lstat(path, &st) < 0)
>> - return process_lstat_error(path, errno);
>> + return process_lstat_error(path);
>>
>> if (S_ISDIR(st.st_mode))
>> return process_directory(path, len, &st);
--
Duy
--
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