[PATCH] realpath stack usage 8k -> 4k

Carmelo AMOROSO carmelo.amoroso at st.com
Tue May 13 07:20:26 PDT 2008


Denys Vlasenko wrote:
> On Thursday 08 May 2008 16:01, Carmelo AMOROSO wrote:
>   
>>>> -      if (!check_path (buf, tests[i].out ? tests[i].out : tests[i].resolved))
>>>> +      if (result && !check_path (buf, tests[i].out ? tests[i].out : tests[i].resolved))
>>>>  	{
>>>>  	  printf ("%s: flunked test %d (expected resolved `%s', got `%s')\n",
>>>>  		  argv[0], i, tests[i].out ? tests[i].out : tests[i].resolved,
>>>>
>>>> So, if realpath return result==NULL (it means it is failing), then
>>>> do not check buffern content and go on to check the errno for ELOOP
>>>>
>>>> Do you agree ?
>>>>     
>>>>         
>>> Yes.
>>>
>>> I committed the fix to svn.
>>>   
>>>       
>> you said yes... but committed something different, you did not use my 
>> last suggestion to use result
>>     
>
> Well, there is a multitude of ways how to do it, all slightly different.
>
> I did not use "if (result && !check_path(...)) construct because it
> does not allow for doing check_path if result is NULL.
> I don't know whether anyone ever realonably _need_ such check.
> I just retained that.
>
> In version which I committed check_path() is done unless
> both expected retval and expected buffer contents is NULL.
> And I set them to NULL in two cases.
> This means that I did minimal change - I switched check off
> ONLY for two cases where it was giving false positives.
>
> I don't feel strongly about it. If you want to switch off
> buffer contents check for all cases where reval is NULL,
> by all means, please do it.
> --
> vda
>   
Hi Denis,
I think that the check against the result is more correct.
 From the man page...
-----------
If there is no error, it returns a pointer to the resolved_path.

       Otherwise it returns a NULL pointer, and the contents of  the  
array  resolved_path  are  undefined.  The
       global variable errno is set to indicate the error.
--------------

I'd prefer to rely on the return value of the function instead of the 
expect values provided by the testcase,
I think it 's clearer from the functional point of view.

If the realpath function returns a not null value while null is 
expected, this is already verified
in the first check (test-canon.c:182 and following)

    182       if (!check_path (result, tests[i].retval))
    183     {
    184       printf ("%s: flunked test %d (expected `%s', got `%s')\n",
    185           argv[0], i, tests[i].retval ? tests[i].retval : "NULL",
    186           result ? result : "NULL");
    187       ++errors;
    188       continue;
    189     }

So, I'm going to change it ;-)

Cheers,
Carmelo



More information about the uClibc mailing list