Re: [PATCH] unpack-trees.c: check return value of lstat()

From: Torsten Bögershausen <tboegi@web.de>
Date: Mar 6, 2011, 19:50 UTC
Message-ID: <4D73E577.1010604@web.de>
In-Reply-To: <vpqfwr0hwu0.fsf@bauges.imag.fr>

On 06.03.11 20:29, Matthieu Moy wrote:

Show 8 quoted lines

> Torsten Bögershausen <tboegi@web.de> writes:
> 
>> +		if (!lstat(path, &st))
>> +			return check_ok_to_remove(path, len, DT_UNKNOWN, NULL, &st,
>> +			                          error_type, o);
>>  	} else if (!lstat(ce->name, &st))
> 
> What happens if lstat returns a non-0 value?

The result of "st" is 100% garbage, and should not be passed to anybody. Not checking the return value (error or not) is not a good thing. Developers might be tempted to write unreliable code...

> 
> Anyway, this seems to have been fixed by a93e53018 (Wed Jan 12 20:28:09
> 2011, unpack-trees: handle lstat failure for existing file) already.
>

Hm,
v1.7.4 says:
commit e39212ab08e8d37dda5d8fd32b54099fe01dbbdb
Merge: 716958c 9e08273
(so far so good)
My git looks like this:

static int verify_absent_1(struct cache_entry *ce,
				 enum unpack_trees_error_types error_type,
				 struct unpack_trees_options *o)
{
[snip]
	else if (len > 0) {
		char path[PATH_MAX + 1];
		memcpy(path, ce->name, len);
		path[len] = 0;
		lstat(path, &st);
                ^^^^^^^^^^^^^^^^^^
		return check_ok_to_remove(path, len, DT_UNKNOWN, NULL, &st,
				error_type, o);

(And I had to correct drizzd@aon.a -> drizzd@aon.at) /Torsten