Re: [LTP] [PATCH v2] readlink03: fix return value for the syscall readlink()

[Wanlong G. <gao...@cn...>]

On 03/15/2012 11:42 AM, Caspar Zhang wrote:

> On 03/14/2012 10:18 PM, Wanlong Gao wrote:
>> Since commit http://git.kernel.org/linus/65cfc672
>> which added O_PATH support to the *at() calls, the error return for readlink/readlinkat
>> for the empty pathname has switched from ENOENT to EINVAL. And this patch merged to linux v2.6.39,
>> So LTP commit http://ltp.git.sourceforge.net/git/gitweb.cgi?p=ltp/ltp.git;a=commitdiff;h=0cc75148b95e2ed8ecb311f20d91f449461d8860
> 
> the URL is too long, let's use a shorter one?
> 
>     https://github.com/linux-test-project/ltp/commit/0cc7514


yeah, better.

> 
>> fix this with the kernel version v2.6.39 compared.
>>
>> But recently, commit http://git.kernel.org/linus/1fa1e7f6
>> which ensure we return ENOENT for the empty pathname for normal lookups.
>>
>> Now, the patch whitch switch the return value back again is already queued for
>> 3.0 and 3.1 stable release.
>>
>> So, if the return value is EINVAL for empty pathname, it may be a kernel bug, so we
>> needn't to check the kernel version but return the "kernel bug" INFO to LTP the users.
> 
> I'd rather a TWARN here, since it's a real kernel bug and a user might
> want to get informed instead of let it hidden inside test log.


exactly.

> 
>>
>> Signed-off-by: Wanlong Gao <gao...@cn...>
>> Acked-by: Jan Stancek <jst...@re...>
>> ---
>>  testcases/kernel/syscalls/readlink/readlink03.c |   10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/readlink/readlink03.c b/testcases/kernel/syscalls/readlink/readlink03.c
>> index 370bf7d..e95078a 100644
>> --- a/testcases/kernel/syscalls/readlink/readlink03.c
>> +++ b/testcases/kernel/syscalls/readlink/readlink03.c
>> @@ -173,11 +173,6 @@ int main(int ac, char **av)
>>  				buf_size = sizeof(buffer);
>>  			}
>>  
>> -			if (strncmp(test_desc, "Symlink Pathname is empty", 25) == 0) {
>> -				if ((tst_kvercmp(2, 6, 39)) >= 0)
>> -					Test_cases[i].exp_errno = EINVAL;
>> -			}
>> -
>>  			/*
>>  			 * Call readlink(2) to test different test conditions.
>>  			 * verify that it fails with -1 return value and sets
>> @@ -204,6 +199,11 @@ int main(int ac, char **av)
>>  					 "errno=%d, expected errno=%d",
>>  					 test_desc, TEST_ERRNO,
>>  					 Test_cases[i].exp_errno);
>> +				if ((strncmp(test_desc, "Symlink Pathname is empty", 25) == 0) &&
>> +				     TEST_ERRNO == EINVAL) {
>> +					printf("It may be a Kernel Bug, see the patch:\n"
>> +					       "http://git.kernel.org/linus/1fa1e7f6\n");
> 
> So we might need to use tst_resm(TWARN here...


Sure, done.

Thanks,
Wanlong Gao

> 
> Thanks,
> Caspar
> 
>> +				}
>>  			}
>>  		}
>>  	}
> 
>