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

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

[Caspar Zhang]

Hi,

On 03/15/2012 11:53 AM, 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 https://github.com/linux-test-project/ltp/commit/0cc7514
> 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.

Hmmm, picky advice: wrapper the line <= 73 character in commit message.

> 
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Acked-by: Jan Stancek <jstancek@redhat.com>
> ---

[snip]

> +				if ((strncmp(test_desc, "Symlink Pathname is empty", 25) == 0) &&
> +				     TEST_ERRNO == EINVAL)
> +					tst_resm(TWARN, "It may be a Kernel Bug, see the patch:\n"

picky advice #2: I think without this \n we can still get a good-looking
warning message.

> +						 "http://git.kernel.org/linus/1fa1e7f6\n");

I guess \n in this line should be removed.

How do you think?

Thanks,
Caspar

------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here 
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

[Wanlong Gao]

On 03/15/2012 12:01 PM, Caspar Zhang wrote:

> Hi,
> 
> On 03/15/2012 11:53 AM, 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 https://github.com/linux-test-project/ltp/commit/0cc7514
>> 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.
> 
> Hmmm, picky advice: wrapper the line <= 73 character in commit message.


OK.

> 
>>
>> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>> Acked-by: Jan Stancek <jstancek@redhat.com>
>> ---
> 
> [snip]
> 
>> +				if ((strncmp(test_desc, "Symlink Pathname is empty", 25) == 0) &&
>> +				     TEST_ERRNO == EINVAL)
>> +					tst_resm(TWARN, "It may be a Kernel Bug, see the patch:\n"
> 
> picky advice #2: I think without this \n we can still get a good-looking
> warning message.
> 
>> +						 "http://git.kernel.org/linus/1fa1e7f6\n");
> 
> I guess \n in this line should be removed.


Sure, I was brain-dead.

Thanks,
Wanlong Gao

> 
> How do you think?
> 
> Thanks,
> Caspar
> 



------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here 
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

[Caspar Zhang]

On 03/15/2012 12:32 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 https://github.com/linux-test-project/ltp/commit/0cc7514
> 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" warning  to LTP the users.
> 
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Acked-by: Jan Stancek <jstancek@redhat.com>

Acked-by: Caspar Zhang <caspar@casparzhang.com>

> ---
>  testcases/kernel/syscalls/readlink/readlink03.c |    9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/readlink/readlink03.c b/testcases/kernel/syscalls/readlink/readlink03.c
> index 370bf7d..5ae8513 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,10 @@ 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)
> +					tst_resm(TWARN, "It may be a Kernel Bug, see the patch:"
> +						 "http://git.kernel.org/linus/1fa1e7f6");
>  			}
>  		}
>  	}


------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here 
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

[Wanlong Gao]

On 03/15/2012 01:22 PM, Caspar Zhang wrote:

> On 03/15/2012 12:32 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 https://github.com/linux-test-project/ltp/commit/0cc7514
>> 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" warning  to LTP the users.
>>
>> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>> Acked-by: Jan Stancek <jstancek@redhat.com>
> 
> Acked-by: Caspar Zhang <caspar@casparzhang.com>


pushed, thanks for your review Caspar.

Regards,
Wanlong Gao

> 
>> ---
>>  testcases/kernel/syscalls/readlink/readlink03.c |    9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/readlink/readlink03.c b/testcases/kernel/syscalls/readlink/readlink03.c
>> index 370bf7d..5ae8513 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,10 @@ 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)
>> +					tst_resm(TWARN, "It may be a Kernel Bug, see the patch:"
>> +						 "http://git.kernel.org/linus/1fa1e7f6");
>>  			}
>>  		}
>>  	}
> 
> 



------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here 
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list