On 06/15/2017 09:34 AM, Alice Ferrazz wrote:
> From: Alice Ferrazzi <alice.ferrazzi@xxxxxxxxx>
>
> 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
Hi Alice,
You sent two patches - they identical. Which one should I be looking
at for review?
I will give you comments on this one assuming this is the right one.
>
> 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..034d3a7263 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");
This change drops the perror() information. What I would do here is
using strerror(errorno) and appending the string.
Another option is making ksft_exit_fail_msg() take var args to take
multiple strings. This will make it easier to pass additional information.
The second option probably is the best as test code can be simpler.
Something to think about even for other kselftest API that prints
messages. I might have more suggestions as I do my test drive porting
breakkpoints_arm64.c
> }
>
> 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");
Same as above. perror() information is lost with this change.
thanks,
-- Shuah
> }
>
> /* An example of output and arguments */
> - printf("pid1: %6d pid2: %6d FD: %2ld FILES: %2ld VM: %2ld "
> + snprintf(buf, 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);
> if (ret) {
> - printf("FAIL: 0 expected but %d returned (%s)\n",
> + snprintf(buf, sizeof(buf), "0 expected but %d returned (%s)",
> ret, strerror(errno));
> - ksft_inc_fail_cnt();
> + ksft_test_result_fail(buf);
> ret = -1;
> } else {
> - printf("PASS: 0 returned as expected\n");
> - ksft_inc_pass_cnt();
> + ksft_test_result_pass("0 returned as expected");
> }
>
> /* 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, sizeof(buf), "0 expected but %d returned (%s)",
> ret, strerror(errno));
> - ksft_inc_fail_cnt();
> + ksft_test_result_fail(buf);
> ret = -1;
> } else {
> - printf("PASS: 0 returned as expected\n");
> - ksft_inc_pass_cnt();
> + ksft_test_result_pass("0 returned as expected");
> }
>
> - 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;
> }
>
--
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
