Re: [musl] realpath without procfs -- should be ready for inclusion

[Alexey Izbyshev <izbyshev@ispras.ru>]

On 2020-11-23 23:53, Rich Felker wrote:
> On Mon, Nov 23, 2020 at 01:56:33PM -0500, Rich Felker wrote:
>> On Sun, Nov 22, 2020 at 10:19:33PM -0500, Rich Felker wrote:
>> --- realpath8.c	2020-11-22 17:52:17.586481571 -0500
>> +++ realpath9.c	2020-11-23 13:55:06.808458893 -0500
>> @@ -19,7 +19,7 @@
>>  	char *output = resolved ? resolved : buf;
>>  	size_t p, q, l, cnt=0;
>> 
>> -	l = strnlen(filename, sizeof stack + 1);
>> +	l = strnlen(filename, sizeof stack);
>>  	if (!l) {
>>  		errno = ENOENT;
>>  		return 0;
>> @@ -80,11 +80,16 @@
>>  			return 0;
>>  		}
>>  		if (k==p) goto toolong;
>> +		if (!k) {
>> +			errno = ENOENT;
>> +			return 0;
>> +		}
>>  		if (++cnt == SYMLOOP_MAX) {
>>  			errno = ELOOP;
>>  			return 0;
>>  		}
>>  		p -= k;
>> +		if (stack[k-1]=='/') p++;
>>  		memmove(stack+p, stack, k);
> 
> This is wrong and needs further consideration.
> 
Yes, now memmove() overwrites NUL if p was at the end and stack[k-1] == 
'/'. Is it true per POSIX that "rr/home" must resolve to "//home" if 
"rr" -> "//"? If so, maybe something like the following instead:

+               while (stack[p] == '/') p++;
+               if (stack[p] && stack[k-1] != '/') p--;
                 p -= k;
-               if (stack[k-1]=='/') p++;

>>  	}
>> 
>> @@ -95,7 +100,8 @@
>>  		l = strlen(stack);
>>  		/* Cancel any initial .. components. */
>>  		p = 0;
>> -		while (q-p>=2 && at_dotdot(output+p+2, p+2)) {
>> +		while (output[p]=='.' && output[p+1]=='.'
>> +		  && (!output[p+2] || output[p+2]=='/')) {
>>  			while(l>1 && stack[l-1]!='/') l--;
>>  			if (l>1) l--;
>>  			p += 2;
> 
> OK, I have a better improvement for this: counting the number of
> levels of .. as they're built at the head of output. Then it's just
> while (nup--) here, and the condition for canceling .. in the first
> loop no longer needs any string inspection; it's just (q>3*nup).
> 
Sounds good.

I've missed the last time that the immediately following code is also 
broken:

>               if (q-p && stack[l-1]!='/') output[--p] = '/';

It will underflow the output in case of a simple relative path that 
doesn't start with "..".

I've also noticed other issues to be fixed, per POSIX:

* ENOENT should be returned if filename is NULL

* ENOTDIR should be returned if the last component is not a directory  
and the path has one or more trailing slashes

Alexey