* [LTP] [PATCH 2/4] syscalls/modify_ldt: Replace TINFO with TPASS or TFAIL
@ 2021-04-29 12:52 Zhao Gongyi
2021-04-30 13:24 ` Cyril Hrubis
2021-05-06 10:44 ` Petr Vorel
0 siblings, 2 replies; 9+ messages in thread
From: Zhao Gongyi @ 2021-04-29 12:52 UTC (permalink / raw)
To: ltp
1)remove redundant Variables flag/fail
2)remove redundant log
3)replace TINFO with TPASS or TFAIL
4)check signal generated by child
For those:
testcases/kernel/syscalls/modify_ldt/modify_ldt01.c
testcases/kernel/syscalls/modify_ldt/modify_ldt02.c
Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
---
.../kernel/syscalls/modify_ldt/modify_ldt01.c | 44 +++++--------------
.../kernel/syscalls/modify_ldt/modify_ldt02.c | 35 ++++-----------
2 files changed, 18 insertions(+), 61 deletions(-)
diff --git a/testcases/kernel/syscalls/modify_ldt/modify_ldt01.c b/testcases/kernel/syscalls/modify_ldt/modify_ldt01.c
index 248111814..86c5b3e5a 100644
--- a/testcases/kernel/syscalls/modify_ldt/modify_ldt01.c
+++ b/testcases/kernel/syscalls/modify_ldt/modify_ldt01.c
@@ -98,7 +98,6 @@ int main(int ac, char **av)
void *ptr;
int retval, func;
- int flag;
int seg[4];
tst_parse_opts(ac, av, NULL, NULL);
@@ -115,8 +114,6 @@ int main(int ac, char **av)
/*
* Check for ENOSYS.
*/
- tst_resm(TINFO, "Enter block 1");
- flag = 0;
ptr = malloc(10);
func = 100;
retval = modify_ldt(func, ptr, sizeof(ptr));
@@ -125,29 +122,21 @@ int main(int ac, char **av)
tst_resm(TFAIL, "modify_ldt() set invalid "
"errno, expected ENOSYS, got: %d",
errno);
- flag = FAILED;
+ } else {
+ tst_resm(TPASS,
+ "modify_ldt() set expected errno");
}
} else {
tst_resm(TFAIL, "modify_ldt error: "
"unexpected return value %d", retval);
- flag = FAILED;
}
- if (flag) {
- tst_resm(TINFO, "block 1 FAILED");
- } else {
- tst_resm(TINFO, "block 1 PASSED");
- }
- tst_resm(TINFO, "Exit block 1");
free(ptr);
//block2:
/*
* Check for EINVAL
*/
- tst_resm(TINFO, "Enter block 2");
- flag = 0;
-
ptr = 0;
retval = modify_ldt(1, ptr, sizeof(ptr));
@@ -156,20 +145,14 @@ int main(int ac, char **av)
tst_resm(TFAIL, "modify_ldt() set invalid "
"errno, expected EINVAL, got: %d",
errno);
- flag = FAILED;
+ } else {
+ tst_resm(TPASS,
+ "modify_ldt() set expected errno");
}
} else {
tst_resm(TFAIL, "modify_ldt error: "
"unexpected return value %d", retval);
- flag = FAILED;
- }
-
- if (flag) {
- tst_resm(TINFO, "block 2 FAILED");
- } else {
- tst_resm(TINFO, "block 2 PASSED");
}
- tst_resm(TINFO, "Exit block 2");
//block3:
@@ -177,7 +160,7 @@ int main(int ac, char **av)
* Create a new LDT segment.
*/
if (create_segment(seg, sizeof(seg)) == -1) {
- tst_brkm(TINFO, cleanup, "Creation of segment failed");
+ tst_brkm(TBROK, cleanup, "Creation of segment failed");
}
/*
@@ -191,21 +174,14 @@ int main(int ac, char **av)
tst_resm(TFAIL, "modify_ldt() set invalid "
"errno, expected EFAULT, got: %d",
errno);
- flag = FAILED;
+ } else {
+ tst_resm(TPASS,
+ "modify_ldt() set expected errno");
}
} else {
tst_resm(TFAIL, "modify_ldt error: "
"unexpected return value %d", retval);
- flag = FAILED;
}
-
- if (flag) {
- tst_resm(TINFO, "block 3 FAILED");
- } else {
- tst_resm(TINFO, "block 3 PASSED");
- }
- tst_resm(TINFO, "Exit block 3");
-
}
cleanup();
tst_exit();
diff --git a/testcases/kernel/syscalls/modify_ldt/modify_ldt02.c b/testcases/kernel/syscalls/modify_ldt/modify_ldt02.c
index c953ac420..769cf6701 100644
--- a/testcases/kernel/syscalls/modify_ldt/modify_ldt02.c
+++ b/testcases/kernel/syscalls/modify_ldt/modify_ldt02.c
@@ -86,15 +86,12 @@ int read_segment(unsigned int);
void cleanup(void);
void setup(void);
-#define FAILED 1
-
int main(int ac, char **av)
{
int lc;
int val, pid, status;
- int flag;
int seg[4];
tst_parse_opts(ac, av, NULL, NULL);
@@ -108,12 +105,9 @@ int main(int ac, char **av)
tst_count = 0;
//block1:
- tst_resm(TINFO, "Enter block 1");
- flag = 0;
-
seg[0] = 12345;
if (create_segment(seg, sizeof(seg)) == -1) {
- tst_brkm(TINFO, cleanup, "Creation of segment failed");
+ tst_brkm(TBROK, cleanup, "Creation of segment failed");
}
val = read_segment(0);
@@ -121,23 +115,12 @@ int main(int ac, char **av)
if (val != seg[0]) {
tst_resm(TFAIL, "Invalid value read %d, expected %d",
val, seg[0]);
- flag = FAILED;
- }
-
- if (flag) {
- tst_resm(TINFO, "block 1 FAILED");
- } else {
- tst_resm(TINFO, "block 1 PASSED");
- }
-
- tst_resm(TINFO, "Exit block 1");
+ } else
+ tst_resm(TPASS, "value read as expected");
//block2:
- tst_resm(TINFO, "Enter block 2");
- flag = 0;
-
if (create_segment(0, 10) == -1) {
- tst_brkm(TINFO, cleanup, "Creation of segment failed");
+ tst_brkm(TBROK, cleanup, "Creation of segment failed");
}
tst_old_flush();
@@ -149,15 +132,13 @@ int main(int ac, char **av)
(void)waitpid(pid, &status, 0);
if (WEXITSTATUS(status) != 0) {
- flag = FAILED;
tst_resm(TFAIL, "Did not generate SEGV, child returned "
"unexpected status");
- }
-
- if (flag) {
- tst_resm(TINFO, "block 2 FAILED");
} else {
- tst_resm(TINFO, "block 2 PASSED");
+ if (WIFSIGNALED(status) && (WTERMSIG(status) == SIGSEGV))
+ tst_resm(TPASS, "generate SEGV as expected");
+ else
+ tst_resm(TFAIL, "Did not generate SEGV");
}
}
cleanup();
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [LTP] [PATCH 2/4] syscalls/modify_ldt: Replace TINFO with TPASS or TFAIL
2021-04-29 12:52 [LTP] [PATCH 2/4] syscalls/modify_ldt: Replace TINFO with TPASS or TFAIL Zhao Gongyi
@ 2021-04-30 13:24 ` Cyril Hrubis
2021-05-06 10:44 ` Petr Vorel
1 sibling, 0 replies; 9+ messages in thread
From: Cyril Hrubis @ 2021-04-30 13:24 UTC (permalink / raw)
To: ltp
Hi!
Pushed with a minor changes, thanks.
I've removed a few useless comments as well, i.e. things that were
commenting the obvious such as "setup() - performs setup" and the
"//block1:" etc.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH 2/4] syscalls/modify_ldt: Replace TINFO with TPASS or TFAIL
2021-04-29 12:52 [LTP] [PATCH 2/4] syscalls/modify_ldt: Replace TINFO with TPASS or TFAIL Zhao Gongyi
2021-04-30 13:24 ` Cyril Hrubis
@ 2021-05-06 10:44 ` Petr Vorel
2021-05-06 11:05 ` Cyril Hrubis
1 sibling, 1 reply; 9+ messages in thread
From: Petr Vorel @ 2021-05-06 10:44 UTC (permalink / raw)
To: ltp
Hi Zhao, Cyril,
> diff --git a/testcases/kernel/syscalls/modify_ldt/modify_ldt01.c b/testcases/kernel/syscalls/modify_ldt/modify_ldt01.c
...
> @@ -149,15 +132,13 @@ int main(int ac, char **av)
> (void)waitpid(pid, &status, 0);
> if (WEXITSTATUS(status) != 0) {
> - flag = FAILED;
> tst_resm(TFAIL, "Did not generate SEGV, child returned "
> "unexpected status");
> - }
> -
> - if (flag) {
> - tst_resm(TINFO, "block 2 FAILED");
> } else {
> - tst_resm(TINFO, "block 2 PASSED");
> + if (WIFSIGNALED(status) && (WTERMSIG(status) == SIGSEGV))
> + tst_resm(TPASS, "generate SEGV as expected");
> + else
> + tst_resm(TFAIL, "Did not generate SEGV");
FYI: since merging this patch (f5e8e6b11) test fails on this. Is it expected?
I haven't looked closer whether it's a test bug or real issue.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 9+ messages in thread* [LTP] [PATCH 2/4] syscalls/modify_ldt: Replace TINFO with TPASS or TFAIL
2021-05-06 10:44 ` Petr Vorel
@ 2021-05-06 11:05 ` Cyril Hrubis
2021-05-06 12:06 ` Petr Vorel
0 siblings, 1 reply; 9+ messages in thread
From: Cyril Hrubis @ 2021-05-06 11:05 UTC (permalink / raw)
To: ltp
Hi!
> FYI: since merging this patch (f5e8e6b11) test fails on this. Is it expected?
> I haven't looked closer whether it's a test bug or real issue.
What is the test failure output? The only part that was change that
could introduce difference in the output is as far as I can tell this:
/*
* Create a new LDT segment.
*/
if (create_segment(seg, sizeof(seg)) == -1) {
- tst_brkm(TINFO, cleanup, "Creation of segment failed");
+ tst_brkm(TBROK, cleanup, "Creation of segment failed");
}
Which would mean that the test was broken before the change but just
haven't produced any errors.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 9+ messages in thread* [LTP] [PATCH 2/4] syscalls/modify_ldt: Replace TINFO with TPASS or TFAIL
2021-05-06 11:05 ` Cyril Hrubis
@ 2021-05-06 12:06 ` Petr Vorel
2021-05-06 11:47 ` Cyril Hrubis
0 siblings, 1 reply; 9+ messages in thread
From: Petr Vorel @ 2021-05-06 12:06 UTC (permalink / raw)
To: ltp
Hi,
> Hi!
> > FYI: since merging this patch (f5e8e6b11) test fails on this. Is it expected?
> > I haven't looked closer whether it's a test bug or real issue.
> What is the test failure output? The only part that was change that
$ ./modify_ldt02
modify_ldt02 1 TPASS : value read as expected
modify_ldt02 0 TINFO : received signal: 11
modify_ldt02 2 TFAIL : modify_ldt02.c:136: Did not generate SEGV
(I wrote my report below affected line)
11 is SIGSEGV, thus there must be a bug in a test, which probably has been there
before (test just didn't check). It's strange that WTERMSIG(status) returns 0
instead of 11.
> could introduce difference in the output is as far as I can tell this:
> /*
> * Create a new LDT segment.
> */
> if (create_segment(seg, sizeof(seg)) == -1) {
> - tst_brkm(TINFO, cleanup, "Creation of segment failed");
> + tst_brkm(TBROK, cleanup, "Creation of segment failed");
> }
> Which would mean that the test was broken before the change but just
> haven't produced any errors.
Yes.
Old results:
modify_ldt02 0 TINFO : Enter block 1
modify_ldt02 0 TINFO : block 1 PASSED
modify_ldt02 0 TINFO : Exit block 1
modify_ldt02 0 TINFO : Enter block 2
modify_ldt02 0 TINFO : received signal: 11
modify_ldt02 0 TINFO : block 2 PASSED
NOTE: no PASS/FAIL.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 9+ messages in thread* [LTP] [PATCH 2/4] syscalls/modify_ldt: Replace TINFO with TPASS or TFAIL
2021-05-06 12:06 ` Petr Vorel
@ 2021-05-06 11:47 ` Cyril Hrubis
0 siblings, 0 replies; 9+ messages in thread
From: Cyril Hrubis @ 2021-05-06 11:47 UTC (permalink / raw)
To: ltp
Hi!
> $ ./modify_ldt02
> modify_ldt02 1 TPASS : value read as expected
> modify_ldt02 0 TINFO : received signal: 11
> modify_ldt02 2 TFAIL : modify_ldt02.c:136: Did not generate SEGV
>
> (I wrote my report below affected line)
>
> 11 is SIGSEGV, thus there must be a bug in a test, which probably has been there
> before (test just didn't check). It's strange that WTERMSIG(status) returns 0
> instead of 11.
Looking at the code the bug is obvious, we setup a handler for SIGSEGV
that does exit(0). And since this is old API test we have to reset the
signal handler in the child as well. I will push a fix ASAP.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH 2/4] syscalls/modify_ldt: Replace TINFO with TPASS or TFAIL
@ 2021-05-06 11:15 zhaogongyi
0 siblings, 0 replies; 9+ messages in thread
From: zhaogongyi @ 2021-05-06 11:15 UTC (permalink / raw)
To: ltp
Hi Petr,
Before my MR merged, the test case would not eixt a nozero value, is it the root cause of the problem?
>
> Hi Zhao, Cyril,
>
> > diff --git a/testcases/kernel/syscalls/modify_ldt/modify_ldt01.c
> b/testcases/kernel/syscalls/modify_ldt/modify_ldt01.c
> ...
> > @@ -149,15 +132,13 @@ int main(int ac, char **av)
> > (void)waitpid(pid, &status, 0);
>
> > if (WEXITSTATUS(status) != 0) {
> > - flag = FAILED;
> > tst_resm(TFAIL, "Did not generate SEGV, child returned "
> > "unexpected status");
> > - }
> > -
> > - if (flag) {
> > - tst_resm(TINFO, "block 2 FAILED");
> > } else {
> > - tst_resm(TINFO, "block 2 PASSED");
> > + if (WIFSIGNALED(status) && (WTERMSIG(status) == SIGSEGV))
> > + tst_resm(TPASS, "generate SEGV as expected");
> > + else
> > + tst_resm(TFAIL, "Did not generate SEGV");
> FYI: since merging this patch (f5e8e6b11) test fails on this. Is it expected?
> I haven't looked closer whether it's a test bug or real issue.
>
> Kind regards,
> Petr
^ permalink raw reply [flat|nested] 9+ messages in thread* [LTP] [PATCH 2/4] syscalls/modify_ldt: Replace TINFO with TPASS or TFAIL
@ 2021-05-06 11:23 zhaogongyi
2021-05-06 11:57 ` Petr Vorel
0 siblings, 1 reply; 9+ messages in thread
From: zhaogongyi @ 2021-05-06 11:23 UTC (permalink / raw)
To: ltp
Hi,
I have reviewed the patch for some time, but I have not find the possible reason of it. Theoretically the patch didn't change the logic.
Because I have not the i386 machine, so I can't run it.
Best Regards,
Gongyi
>
> Hi Petr,
>
> Before my MR merged, the test case would not eixt a nozero value, is it
> the root cause of the problem?
>
> >
> > Hi Zhao, Cyril,
> >
> > > diff --git a/testcases/kernel/syscalls/modify_ldt/modify_ldt01.c
> > b/testcases/kernel/syscalls/modify_ldt/modify_ldt01.c
> > ...
> > > @@ -149,15 +132,13 @@ int main(int ac, char **av)
> > > (void)waitpid(pid, &status, 0);
> >
> > > if (WEXITSTATUS(status) != 0) {
> > > - flag = FAILED;
> > > tst_resm(TFAIL, "Did not generate SEGV, child returned "
> > > "unexpected status");
> > > - }
> > > -
> > > - if (flag) {
> > > - tst_resm(TINFO, "block 2 FAILED");
> > > } else {
> > > - tst_resm(TINFO, "block 2 PASSED");
> > > + if (WIFSIGNALED(status) && (WTERMSIG(status) ==
> SIGSEGV))
> > > + tst_resm(TPASS, "generate SEGV as expected");
> > > + else
> > > + tst_resm(TFAIL, "Did not generate SEGV");
> > FYI: since merging this patch (f5e8e6b11) test fails on this. Is it
> expected?
> > I haven't looked closer whether it's a test bug or real issue.
> >
> > Kind regards,
> > Petr
^ permalink raw reply [flat|nested] 9+ messages in thread* [LTP] [PATCH 2/4] syscalls/modify_ldt: Replace TINFO with TPASS or TFAIL
2021-05-06 11:23 zhaogongyi
@ 2021-05-06 11:57 ` Petr Vorel
0 siblings, 0 replies; 9+ messages in thread
From: Petr Vorel @ 2021-05-06 11:57 UTC (permalink / raw)
To: ltp
HI Gongyi,
> I have reviewed the patch for some time, but I have not find the possible reason of it. Theoretically the patch didn't change the logic.
I don't see the problem on first quick look.
> Because I have not the i386 machine, so I can't run it.
FYI You can compile 32bit on 64bit machine:
$ PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig CFLAGS=-m32 LDFLAGS=-m32 ./configure \
&& make && make install
NOTE: PKG_CONFIG_LIBDIR can vary, see INSTALL file. Or you can build LTP 32bit
with build.sh script:
$ ./build.sh -t 32
Kind regards,
Petr
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-05-06 12:06 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-04-29 12:52 [LTP] [PATCH 2/4] syscalls/modify_ldt: Replace TINFO with TPASS or TFAIL Zhao Gongyi
2021-04-30 13:24 ` Cyril Hrubis
2021-05-06 10:44 ` Petr Vorel
2021-05-06 11:05 ` Cyril Hrubis
2021-05-06 12:06 ` Petr Vorel
2021-05-06 11:47 ` Cyril Hrubis
-- strict thread matches above, loose matches on Subject: below --
2021-05-06 11:15 zhaogongyi
2021-05-06 11:23 zhaogongyi
2021-05-06 11:57 ` Petr Vorel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox