Re: [PATCH v2] add, rm, mv: fix bug that prevents the update of non-sparse dirs

From: Derrick Stolee <stolee@gmail.com>
Date: Oct 26, 2021, 19:04 UTC
Message-ID: <00a67af9-da41-6df4-afc0-5ae7c7714bfd@gmail.com>
In-Reply-To: <ca1c6a86-23ab-57ae-b1ca-64a9851d72db@web.de>

On 10/26/2021 12:22 PM, René Scharfe wrote:

> Am 25.10.21 um 23:07 schrieb Matheus Tavares:

I reordered some things to first audit that 'slash' is used safely, assuming that we can store "p - 1" if p is a non-zero pointer.

Show 7 quoted lines

>> +	/*
>> +	 * If UNDECIDED, use the match from the parent dir (recursively),
>> +	 * or fall back to NOT_MATCHED at the topmost level.
>> +	 */
>> +	for (end = path + strlen(path); end > path && match == UNDECIDED; end = slash) {
>> +
>> +		for (slash = end - 1; slash >= path && *slash != '/'; slash--)

Since "slash >= path" is compared before dereferencing '*slash', this is safe.

>> +			; /* do nothing */
>

>> +
>> +		match = path_matches_pattern_list(path, end - path,
>> +				slash >= path ? slash + 1 : path, &dtype,

This is also a safe use of 'slash'.

Show 5 quoted lines

> slash can end up one less than path.  If path points to the first char
> of a string object this would be undefined if I read 6.5.6 of C99
> correctly.  (A pointer to the array element just after the last one is
> specified as fine as long as it's not dereferenced, but a pointer to
> the element before the first one is not mentioned and thus undefined.)

I also see the specification saying this is undefined, but I do not understand how any reasonable compiler/runtime could do anything other than store "path - 1" as if it was an unsigned integer. There are a lot of references about "the array" that the pointer points to, but these pointer arithmetic things are not actually accessing the memory allocator.

> Do you really need the ">=" instead of ">"?

I think the only case that would be of any interest is if the path started with a slash, which would not be a valid worktree path. I believe we could use ">" for an abundance of caution with the undefined nature of subtracting from a pointer, but it is non- obvious that that is a real problem.

Thanks, -Stolee