On Wed, Jun 14, 2017 at 11:30:51PM +0000, Bird, Timothy wrote:
> > -----Original Message-----
> > From: Alice Ferrazz on Thursday, June 15, 2017 7:33 AM
> >
> > Make the kcmp test output in the TAP13 format by using the
> > TAP13 output functions defined in kselftest.h
> >
> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Paul Elder <paul.elder@xxxxxxxx>
> > Signed-off-by: Alice Ferrazzi <alice.ferrazzi@xxxxxxxxx>
> > ---
> > v3: Add kcmp test port to TAP13 - Alice
> >
> > tools/testing/selftests/kcmp/kcmp_test.c | 33 ++++++++++++++-------------
> > -----
> > 1 file changed, 14 insertions(+), 19 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kcmp/kcmp_test.c
> > b/tools/testing/selftests/kcmp/kcmp_test.c
> > index a5a4da856d..b007e62e03 100644
> > --- a/tools/testing/selftests/kcmp/kcmp_test.c
> > +++ b/tools/testing/selftests/kcmp/kcmp_test.c
> > @@ -35,30 +35,28 @@ int main(int argc, char **argv)
> > pid1 = getpid();
> >
> > if (fd1 < 0) {
> > - perror("Can't create file");
> > - ksft_exit_fail();
> > + ksft_exit_fail_msg("Can't create file");
> > }
> >
> > pid2 = fork();
> > if (pid2 < 0) {
> > - perror("fork failed");
> > - ksft_exit_fail();
> > + ksft_exit_fail_msg("fork failed");
> > }
> >
> > if (!pid2) {
> > int pid2 = getpid();
> > int ret;
> > + char buf[512];
> >
> > fd2 = open(kpath, O_RDWR, 0644);
> > if (fd2 < 0) {
> > - perror("Can't open file");
> > - ksft_exit_fail();
> > + ksft_exit_fail_msg("Can't open file");
> > }
> >
> > /* An example of output and arguments */
> > - printf("pid1: %6d pid2: %6d FD: %2ld FILES: %2ld VM: %2ld "
> > + snprintf(&buf[0], sizeof(buf) ,"pid1: %6d pid2: %6d FD: %2ld
> > FILES: %2ld VM: %2ld "
> > "FS: %2ld SIGHAND: %2ld IO: %2ld SYSVSEM: %2ld "
> > - "INV: %2ld\n",
> > + "INV: %2ld",
> > pid1, pid2,
> > sys_kcmp(pid1, pid2, KCMP_FILE, fd1, fd2),
> > sys_kcmp(pid1, pid2, KCMP_FILES, 0, 0),
> > @@ -73,30 +71,27 @@ int main(int argc, char **argv)
> >
> > /* This one should return same fd */
> > ret = sys_kcmp(pid1, pid2, KCMP_FILE, fd1, fd1);
> > + ksft_test_result_pass(buf);
> This uses a string with all kinds of dynamic data (particularly pid
> values) as the test description. Please use only non-dynamic
> strings for test descriptions. If it is desired to output information about
> the data used for the test, that should be in a YAML block.
>
> > if (ret) {
> > - printf("FAIL: 0 expected but %d returned (%s)\n",
> > + snprintf(&buf[0], sizeof(buf), "0 expected but %d
> > returned (%s)",
> > ret, strerror(errno));
> > - ksft_inc_fail_cnt();
> > + ksft_test_result_fail(buf);
> This is using test results in place of the test description. Please output
> test results information and explanation AFTER the test description line.
>
> > ret = -1;
> > } else {
> > - printf("PASS: 0 returned as expected\n");
> > - ksft_inc_pass_cnt();
> > + ksft_test_result_pass("0 returned as expected");
> This is not a good test description. It describes results rather than the
> test case.
>
> > }
> >
> > /* Compare with self */
> > ret = sys_kcmp(pid1, pid1, KCMP_VM, 0, 0);
> > if (ret) {
> > - printf("FAIL: 0 expected but %d returned (%s)\n",
> > + snprintf(&buf[0], sizeof(buf), "0 expected but %d
> > returned (%s)",
> > ret, strerror(errno));
> > - ksft_inc_fail_cnt();
> > + ksft_test_result_fail(buf);
> Same issue as above here. Please do not use dynamic strings with results
> as the test description.
>
> > ret = -1;
> > } else {
> > - printf("PASS: 0 returned as expected\n");
> > - ksft_inc_pass_cnt();
> > + ksft_test_result_pass("0 returned as expected");
> Same issue here.
> > }
> >
> > - ksft_print_cnts();
> > -
> > if (ret)
> > ksft_exit_fail();
> > else
> > @@ -105,5 +100,5 @@ int main(int argc, char **argv)
> >
> > waitpid(pid2, &status, P_ALL);
> >
> > - return ksft_exit_pass();
> > + return 0;
> The return should be removed, and this should be just a call to:
> ksft_exit_pass();
>
> Or, this should be left alone. Shuah said she would review these
> calls to see if the return was still warranted.
>
> The ksft_exit_pass() routine is there to provide a level of abstraction
> (e.g. in case the exit codes change in the future), so it should not
> be removed.
The problem came with the fork of this script at line 41.
Because of the fork pid1 die at line 103 and pid2 die at line 98.
Because both are using ksft_exit they will both use the
ksft_print_cnts function resulting in this:
pid2: 1..3
pid1: 1..0
Here the problem other than printing two time the test result is also
that pid1 dosen't know of the test done by pid2.
resulting in 1..0
> -- Tim
>
>
> > }
> > --
> > 2.13.0
> >
>
Thanks,
Alice
Attachment:
signature.asc
Description: PGP signature
