> -----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. -- Tim > } > -- > 2.13.0 > -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html