readlink03: fix return value for the syscall readlink()

PATCH Caspar Zhang 3 participants 4 messages Mar 15, 2012 3 messages not in the archive

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Caspar Zhang

author

2012-03-15 04:03 UTC #2 lore raw

Hi,

Quoted 6 lines · Wanlong Gao

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.

Quoted 8 lines

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.

Quoted 4 lines

Signed-off-by Wanlong Gao <[email protected]>
Acked-by Jan Stancek <[email protected]>
---

[snip]

Review

Quoted 3 lines

(quoted diff) +3 -0

+				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.

Review

Quoted 1 line

(quoted diff) +1 -0

+						 "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
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Wanlong Gao

2012-03-15 04:48 UTC #3 lore raw

Quoted 12 lines · Caspar Zhang

Hi,
Quoted 9 lines · Wanlong Gao
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.

Quoted 7 lines

Quoted 5 lines
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.

Quoted 15 lines

Quoted 5 lines
Signed-off-by Wanlong Gao <[email protected]>
Acked-by Jan Stancek <[email protected]>
---
[snip]
Review
Quoted 3 lines
(quoted diff) +3 -0
+				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.

Quoted 3 lines

Review
Quoted 1 line
(quoted diff) +1 -0
+						 "http://git.kernel.org/linus/1fa1e7f6\n");
I guess \n in this line should be removed.

Sure, I was brain-dead.

Thanks,
Wanlong Gao

Quoted 6 lines

How do you think?
Thanks,
Caspar

A message that is not in the archive (sent off-list or not yet received): [email protected]

Caspar Zhang

Acked-by

2012-03-15 05:23 UTC #5 lore raw

Quoted 15 lines · Wanlong Gao

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.

Quoted 6 lines

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 <[email protected]>
Acked-by Jan Stancek <[email protected]>

Acked-by Caspar Zhang <[email protected]>

Quoted 21 lines

---
 testcases/kernel/syscalls/readlink/readlink03.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

testcases/kernel/syscalls/readlink/readlink03.c +0 -5

index 370bf7d..5ae8513 100644
@@ -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

testcases/kernel/syscalls/readlink/readlink03.c int main(int ac, char **av) · line 208

Quoted 10 lines

testcases/kernel/syscalls/readlink/readlink03.c +4 -0

@@ -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");
 			}
 		}
 	}

Wanlong Gao

2012-03-15 05:48 UTC #6 lore raw

Quoted 16 lines · Caspar Zhang

Quoted 15 lines · Wanlong Gao
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.

Quoted 8 lines

Quoted 6 lines
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 <[email protected]>
Acked-by Jan Stancek <[email protected]>
Acked-by Caspar Zhang <[email protected]>

pushed, thanks for your review Caspar.

Regards,
Wanlong Gao

Quoted 24 lines

---
 testcases/kernel/syscalls/readlink/readlink03.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

testcases/kernel/syscalls/readlink/readlink03.c +0 -5

index 370bf7d..5ae8513 100644
@@ -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,

Quoted 10 lines

testcases/kernel/syscalls/readlink/readlink03.c int main(int ac, char **av) · line 208

Quoted 8 lines

testcases/kernel/syscalls/readlink/readlink03.c +4 -0

 					 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");
 			}
 		}
 	}