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

[Derrick Stolee <stolee@gmail.com>]

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.

>> +	/*
>> +	 * 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'.

> 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