[LTP] [PATCH v3 1/2] lib: move create_sig_proc() into newlib

[LTP] [PATCH v3 1/2] lib: move create_sig_proc() into newlib

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 include/tst_sig_proc.h                    | 22 +++++++++++++-
 lib/tst_sig_proc.c                        | 40 ++++++++++++++++++++++++-
 testcases/kernel/syscalls/ppoll/ppoll01.c | 20 +------------
 3 files changed, 63 insertions(+), 19 deletions(-)
 create mode 100644 include/tst_sig_proc.h
 create mode 100644 lib/tst_sig_proc.c

diff --git a/include/tst_sig_proc.h b/include/tst_sig_proc.h
new file mode 100644
index 0000000..fb7f93f
--- /dev/null
+++ b/include/tst_sig_proc.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2016 Linux Test Project
+ *
+ * This program is free software;  you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY;  without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+ * the GNU General Public License for more details.
+ */
+
+#ifndef TST_SIG_PROC_H__
+#define TST_SIG_PROC_H__
+
+#include <sys/types.h>
+
+pid_t create_sig_proc(int sig, int count, unsigned int usec);
+
+#endif	/* TST_SIG_PROC_H__ */
diff --git a/lib/tst_sig_proc.c b/lib/tst_sig_proc.c
new file mode 100644
index 0000000..579047a
--- /dev/null
+++ b/lib/tst_sig_proc.c
@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2016 Linux Test Project
+ *
+ * This program is free software;  you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY;  without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+ * the GNU General Public License for more details.
+ */
+
+#include <stdlib.h>
+#include <sys/types.h>
+
+#include "tst_sig_proc.h"
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+
+pid_t create_sig_proc(int sig, int count, unsigned int usec)
+{
+	pid_t pid, cpid;
+
+	pid = getpid();
+	cpid = SAFE_FORK();
+
+	if (cpid == 0) {
+		while (count-- > 0) {
+			usleep(usec);
+			if (kill(pid, sig) == -1)
+				break;
+		}
+		exit(0);
+	}
+
+	return cpid;
+}
diff --git a/testcases/kernel/syscalls/ppoll/ppoll01.c b/testcases/kernel/syscalls/ppoll/ppoll01.c
index a5815b3..8d6d7e7 100644
--- a/testcases/kernel/syscalls/ppoll/ppoll01.c
+++ b/testcases/kernel/syscalls/ppoll/ppoll01.c
@@ -28,6 +28,7 @@
 #include <sys/wait.h>
 #include "linux_syscall_numbers.h"
 #include "ltp_signal.h"
+#include "tst_sig_proc.h"
 #include "tst_test.h"
 
 /* Older versions of glibc don't publish this constant's value. */
@@ -205,25 +206,6 @@ static void cleanup(void)
 		close(fd1);
 }
 
-static pid_t create_sig_proc(int sig, int count, unsigned int usec)
-{
-	pid_t pid, cpid;
-
-	pid = getpid();
-	cpid = SAFE_FORK();
-
-	if (cpid == 0) {
-		while (count-- > 0) {
-			usleep(usec);
-			if (kill(pid, sig) == -1)
-				break;
-		}
-		exit(0);
-	}
-
-	return cpid;
-}
-
 static void do_test(unsigned int i)
 {
 	pid_t pid = 0;

base-commit: 437cbaeee5dfafb8e371fb3b2731d4f2e720258f
-- 
git-series 0.9.1

[LTP] [PATCH v3 2/2] testcases/clock_nanosleep01: convert to use new test library API

[Petr Vorel]

removed test type NULL_POINTER

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v2->v3:
* measuring elapsed time with CLOCK_MONOTONIC clock (previous was with clock
  for particular test)
* cleanup (embed the struct timespec into the test structure, SAFE_KILL, use
  TTERRNO, simplify result reporting, ...)
* remove redundant -lrt
* add missing disclamer paragraph (GPL)

Changes v1->v2:
* use helper functions for measuring elapsed time
* removed unused variables
* renaming variables
* removed useless comments
* removed use of temporary directory
---
 testcases/kernel/syscalls/clock_nanosleep/clock_nanosleep01.c | 491 +------
 1 file changed, 132 insertions(+), 359 deletions(-)

diff --git a/testcases/kernel/syscalls/clock_nanosleep/clock_nanosleep01.c b/testcases/kernel/syscalls/clock_nanosleep/clock_nanosleep01.c
index 08c1fbd..1aab77e 100644
--- a/testcases/kernel/syscalls/clock_nanosleep/clock_nanosleep01.c
+++ b/testcases/kernel/syscalls/clock_nanosleep/clock_nanosleep01.c
@@ -1,158 +1,53 @@
-/******************************************************************************/
-/* Copyright (c) Crackerjack Project., 2007-2008 ,Hitachi, Ltd		*/
-/*	  Author(s): Takahiro Yasui <takahiro.yasui.mp@hitachi.com>,	      */
-/*		       Yumiko Sugita <yumiko.sugita.yf@hitachi.com>, 	      */
-/*		       Satoshi Fujiwara <sa-fuji@sdl.hitachi.co.jp>	      */
-/*								  	      */
-/* This program is free software;  you can redistribute it and/or modify      */
-/* it under the terms of the GNU General Public License as published by       */
-/* the Free Software Foundation; either version 2 of the License, or	  */
-/* (at your option) any later version.					*/
-/*									    */
-/* This program is distributed in the hope that it will be useful,	    */
-/* but WITHOUT ANY WARRANTY;  without even the implied warranty of	    */
-/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See		  */
-/* the GNU General Public License for more details.			   */
-/*									    */
-/* You should have received a copy of the GNU General Public License	  */
-/* along with this program;  if not, write to the Free Software	       */
-/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    */
-/*									    */
-/******************************************************************************/
-/******************************************************************************/
-/*									    */
-/* File:	clock_nanosleep01.c					   */
-/*									    */
-/* Description: This tests the clock_nanosleep() syscall		      */
-/*									      */
-/* 									      */
-/*									      */
-/*									      */
-/*									      */
-/*									    */
-/* Usage:  <for command-line>						 */
-/* clock_nanosleep01 [-c n] [-e][-i n] [-I x] [-p x] [-t]		     */
-/*      where,  -c n : Run n copies concurrently.			     */
-/*	      -e   : Turn on errno logging.				 */
-/*	      -i n : Execute test n times.				  */
-/*	      -I x : Execute test for x seconds.			    */
-/*	      -P x : Pause for x seconds between iterations.		*/
-/*	      -t   : Turn on syscall timing.				*/
-/*									    */
-/* Total Tests: 1							     */
-/*									    */
-/* Test Name:   clock_nanosleep01					     */
-/* History:     Porting from Crackerjack to LTP is done by		    */
-/*	      Manas Kumar Nayak maknayak@in.ibm.com>			*/
-/******************************************************************************/
-#include <sys/syscall.h>
-#include <sys/types.h>
-#include <getopt.h>
-#include <string.h>
-#include <stdlib.h>
-#include <errno.h>
-#include <stdio.h>
-#include <time.h>
-#include <signal.h>
-#include "../utils/common_j_h.c"
-#include "../utils/include_j_h.h"
-
-#include "test.h"
-#include "linux_syscall_numbers.h"
-
-char *TCID = "clock_nanosleep01";
-int testno;
-int TST_TOTAL = 1;
-struct sigaction act;
-
 /*
- * sighandler()
+ * Copyright (c) Crackerjack Project., 2007-2008 ,Hitachi, Ltd
+ *          Author(s): Takahiro Yasui <takahiro.yasui.mp@hitachi.com>,
+ *		       Yumiko Sugita <yumiko.sugita.yf@hitachi.com>,
+ *		       Satoshi Fujiwara <sa-fuji@sdl.hitachi.co.jp>
+ * Copyright (c) 2016 Linux Test Project
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
-void sighandler(int sig)
-{
-	if (sig == SIGINT)
-		return;
-
-	return;
-}
 
-/* Extern Global Functions */
-/******************************************************************************/
-/*									    */
-/* Function:    cleanup						       */
-/*									    */
-/* Description: Performs all one time clean up for this test on successful    */
-/*	      completion,  premature exit or  failure. Closes all temporary */
-/*	      files, removes all temporary directories exits the test with  */
-/*	      appropriate return code by calling tst_exit() function.       */
-/*									    */
-/* Input:       None.							 */
-/*									    */
-/* Output:      None.							 */
-/*									    */
-/* Return:      On failure - Exits calling tst_exit(). Non '0' return code.   */
-/*	      On success - Exits calling tst_exit(). With '0' return code.  */
-/*									    */
-/******************************************************************************/
-void cleanup(void)
-{
+#include <limits.h>
 
-	tst_rmdir();
+#include "linux_syscall_numbers.h"
+#include "tst_sig_proc.h"
+#include "tst_timer.h"
+#include "tst_test.h"
 
-}
+#define MAX_MSEC_DIFF   20
 
-/* Local  Functions */
-/******************************************************************************/
-/*									    */
-/* Function:    setup							 */
-/*									    */
-/* Description: Performs all one time setup for this test. This function is   */
-/*	      typically used to capture signals, create temporary dirs      */
-/*	      and temporary files that may be used in the course of this    */
-/*	      test.							 */
-/*									    */
-/* Input:       None.							 */
-/*									    */
-/* Output:      None.							 */
-/*									    */
-/* Return:      On failure - Exits by calling cleanup().		      */
-/*	      On success - returns 0.				       */
-/*									    */
-/******************************************************************************/
-void setup(void)
+static void sighandler(int sig LTP_ATTRIBUTE_UNUSED)
 {
-	/* Capture signals if any */
-	act.sa_handler = sighandler;
-	sigfillset(&act.sa_mask);
-	sigaction(SIGINT, &act, NULL);
-
-	/* Create temporary directories */
-	TEST_PAUSE;
-	tst_tmpdir();
 }
 
-/*
- * Macros
- */
-#define SYSCALL_NAME    "clock_nanosleep"
-
 enum test_type {
 	NORMAL,
-	NULL_POINTER,
 	SEND_SIGINT,
 };
 
-/*
- * Data Structure
- */
+#define TYPE_NAME(x) .ttype = x, .desc = #x
+
 struct test_case {
-	clockid_t clk_id;
-	int ttype;
-	int flags;
-	time_t sec;
-	long nsec;
-	int ret;
-	int err;
+	clockid_t clk_id;	   /* clock_* clock type parameter */
+	int ttype;		   /* test type (enum) */
+	const char *desc;	   /* test description (name) */
+	int flags;		   /* clock_nanosleep flags parameter */
+	struct timespec rq;
+	int exp_ret;
+	int exp_err;
 };
 
 /* Test cases
@@ -161,245 +56,123 @@ struct test_case {
  *
  *   EINTR	      v (function was interrupted by a signal)
  *   EINVAL	     v (invalid tv_nsec, etc.)
- *   ENOTSUP	    can't check because not supported clk_id generates
- *		      EINVAL
+ *   ENOTSUP	    can't check because not supported clk_id generates EINVAL
  */
 
 static struct test_case tcase[] = {
-	{			// case00
-	 .clk_id = CLOCK_REALTIME,
-	 .ttype = NORMAL,
-	 .flags = 0,
-	 .sec = 0,
-	 .nsec = 500000000,	// 500msec
-	 .ret = 0,
-	 .err = 0,
-	 },
-	{			// case01
-	 .clk_id = CLOCK_MONOTONIC,
-	 .ttype = NORMAL,
-	 .flags = 0,
-	 .sec = 0,
-	 .nsec = 500000000,	// 500msec
-	 .ret = 0,
-	 .err = 0,
-	 },
-	{			// case02
-	 .ttype = NORMAL,
-	 .clk_id = CLOCK_REALTIME,
-	 .flags = 0,
-	 .sec = 0,
-	 .nsec = -1,		// invalid
-	 .ret = EINVAL,
-	 .err = 0,
-	 },
-	{			// case03
-	 .ttype = NORMAL,
-	 .clk_id = CLOCK_REALTIME,
-	 .flags = 0,
-	 .sec = 0,
-	 .nsec = 1000000000,	// invalid
-	 .ret = EINVAL,
-	 .err = 0,
-	 },
-	{			// case04
-	 .ttype = NORMAL,
-	 .clk_id = CLOCK_THREAD_CPUTIME_ID,	// not supported
-	 .flags = 0,
-	 .sec = 0,
-	 .nsec = 500000000,	// 500msec
-	 .ret = EINVAL,		// RHEL4U1 + 2.6.18 returns EINVAL
-	 .err = 0,
-	 },
-	{			// case05
-	 .ttype = SEND_SIGINT,
-	 .clk_id = CLOCK_REALTIME,
-	 .flags = 0,
-	 .sec = 10,
-	 .nsec = 0,
-	 .ret = EINTR,
-	 .err = 0,
-	 },
-#if 0				// glibc generates SEGV error (RHEL4U1 + 2.6.18)
-	{			// caseXX
-	 .ttype = NULL_POINTER,
-	 .clk_id = CLOCK_REALTIME,
-	 .flags = 0,
-	 .sec = 0,
-	 .nsec = 500000000,	// 500msec
-	 .ret = EFAULT,
-	 .err = 0,
-	 },
-#endif
+	{
+		.clk_id = CLOCK_REALTIME,
+		TYPE_NAME(NORMAL),
+		.flags = 0,
+		.rq = (struct timespec) {.tv_sec = 0, .tv_nsec = 500000000},
+		.exp_ret = 0,
+		.exp_err = 0,
+	},
+	{
+		.clk_id = CLOCK_MONOTONIC,
+		TYPE_NAME(NORMAL),
+		.flags = 0,
+		.rq = (struct timespec) {.tv_sec = 0, .tv_nsec = 500000000},
+		.exp_ret = 0,
+		.exp_err = 0,
+	},
+	{
+		TYPE_NAME(NORMAL),
+		.clk_id = CLOCK_REALTIME,
+		.flags = 0,
+		.rq = (struct timespec) {.tv_sec = 0, .tv_nsec = -1},
+		.exp_ret = EINVAL,
+		.exp_err = 0,
+	},
+	{
+		TYPE_NAME(NORMAL),
+		.clk_id = CLOCK_REALTIME,
+		.flags = 0,
+		.rq = (struct timespec) {.tv_sec = 0, .tv_nsec = 1000000000},
+		.exp_ret = EINVAL,
+		.exp_err = 0,
+	},
+	{
+		TYPE_NAME(NORMAL),
+		.clk_id = CLOCK_THREAD_CPUTIME_ID,
+		.flags = 0,
+		.rq = (struct timespec) {.tv_sec = 0, .tv_nsec = 500000000},
+		.exp_ret = EINVAL,
+		.exp_err = 0,
+	},
+	{
+		TYPE_NAME(SEND_SIGINT),
+		.clk_id = CLOCK_REALTIME,
+		.flags = 0,
+		.rq = (struct timespec) {.tv_sec = 10, .tv_nsec = 0},
+		.exp_ret = EINTR,
+		.exp_err = 0,
+	},
 };
 
-/*
- * chk_difftime()
- *   Return : OK(0), NG(-1)
- */
-#define MAX_MSEC_DIFF   20
-
-static int chk_difftime(struct timespec *bef, struct timespec *aft,
-			time_t sec, long nsec)
+void setup(void)
 {
-	struct timespec t;
-	time_t expect;
-	time_t result;
-
-	t.tv_sec = aft->tv_sec - bef->tv_sec;
-	t.tv_nsec = aft->tv_nsec - bef->tv_nsec;
-	if (t.tv_nsec < 0) {
-		t.tv_sec -= 1;
-		t.tv_nsec += 1000000000;
-	}
-	expect = (sec * 1000) + (nsec / 1000000);
-	result = (t.tv_sec * 1000) + (t.tv_nsec / 1000000);
-	tst_resm(TINFO, "check sleep time: (min:%ld) < %ld < (max:%ld) (msec)",
-		 expect - MAX_MSEC_DIFF, result, expect + MAX_MSEC_DIFF);
-	if (result < expect - MAX_MSEC_DIFF || result > expect + MAX_MSEC_DIFF)
-		return -1;
-	return 0;
+	SAFE_SIGNAL(SIGINT, sighandler);
+	tst_timer_check(CLOCK_MONOTONIC);
 }
 
-/*
- * do_test()
- *
- *   Input  : TestCase Data
- *   Return : RESULT_OK(0), RESULT_NG(1)
- *
- */
-static int do_test(struct test_case *tc)
+static void do_test(unsigned int i)
 {
-	int sys_ret;
-	int sys_errno;
-	int result = RESULT_OK;
-	struct timespec beftp, afttp, rq, rm;
-	int rc, range_ok = 1, remain_ok = 1;
+	struct test_case *tc = &tcase[i];
+	struct timespec rm;
+	long long elapsed_ms = 0, expect_ms = 0;
 	pid_t pid = 0;
 
-	/*
-	 * Check before sleep time
-	 */
+	tst_res(TINFO, "case %s", tc->desc);
+
+	/* setup */
 	if (tc->ttype == SEND_SIGINT) {
-		pid = create_sig_proc(500000, SIGINT, UINT_MAX);
-		if (pid < 0)
-			return 1;
+		pid = create_sig_proc(SIGINT, 40, 500000);
 	}
 
-	/*
-	 * Check before sleep time
-	 */
-	TEST(rc = clock_gettime(tc->clk_id, &beftp));
-	if (rc < 0) {
-		tst_resm(TFAIL | TTERRNO, "clock_gettime failed");
-		result = 1;
-		goto EXIT;
-	}
-	/*
-	 * Execute system call
-	 */
-	rq.tv_sec = tc->sec;
-	rq.tv_nsec = tc->nsec;
-	// !!!CAUTION: 'clock_gettime' returns errno itself
-	errno = 0;
-	if (tc->ttype == NULL_POINTER)
-		TEST(sys_ret =
-		     clock_nanosleep(tc->clk_id, tc->flags, NULL, &rm));
-	else
-		TEST(sys_ret =
-		     clock_nanosleep(tc->clk_id, tc->flags, &rq, &rm));
-	sys_errno = errno;
-	if (sys_ret != 0)
-		goto TEST_END;
+	/* test */
+	tst_timer_start(CLOCK_MONOTONIC);
+	TEST(clock_nanosleep(tc->clk_id, tc->flags, &tc->rq, &rm));
+	tst_timer_stop();
 
-	/*
-	 * Check after sleep time
-	 */
-	TEST(rc = clock_gettime(tc->clk_id, &afttp));
-	if (TEST_RETURN < 0) {
-		EPRINTF("clock_gettime failed.\n");
-		result = 1;
-		goto EXIT;
-	}
-	range_ok = chk_difftime(&beftp, &afttp, tc->sec, tc->nsec) == 0;
-	/*
-	 * Check remaining time
-	 */
-TEST_END:
-	if (tc->ttype == NORMAL || tc->ttype == SEND_SIGINT) {
-		tst_resm(TINFO, "remain time: %ld %ld", rm.tv_sec, rm.tv_nsec);
-		if (tc->ttype == NORMAL)
-			remain_ok = 1;
-		else
-			remain_ok = rm.tv_sec != 0 || rm.tv_nsec != 0;
+	if (!TEST_RETURN) {
+		expect_ms = tst_timespec_to_ms(tc->rq);
+		elapsed_ms = tst_timer_elapsed_ms();
 	}
 
-	/*
-	 * Check results
-	 */
-	result |= (sys_ret != tc->ret) || !range_ok || !remain_ok;
-	if (!range_ok)
-		PRINT_RESULT_EXTRA(0, tc->ret, tc->err, sys_ret, sys_errno,
-				   "time range check", range_ok);
-	else
-		PRINT_RESULT_EXTRA(0, tc->ret, tc->err, sys_ret, sys_errno,
-				   "remain time check", remain_ok);
-EXIT:
+	tst_res(TINFO, "remain time: %ld %ld", rm.tv_sec, rm.tv_nsec);
+
+	/* cleanup */
 	if (pid > 0) {
-		int st;
-		TEST(kill(pid, SIGTERM));
-		TEST(wait(&st));
+		SAFE_KILL(pid, SIGTERM);
+		SAFE_WAIT(NULL);
 	}
-	return result;
-}
-
-/*
- * main()
- */
-
-int main(int ac, char **av)
-{
-	int result = RESULT_OK;
-	int i;
-	int lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); ++lc) {
-
-		tst_count = 0;
-
-		for (testno = 0; testno < TST_TOTAL; ++testno) {
-
-			for (i = 0; i < (int)(sizeof(tcase) / sizeof(tcase[0]));
-			     i++) {
-				int ret;
-				tst_resm(TINFO, "(case%02d) START", i);
-				ret = do_test(&tcase[i]);
-				tst_resm(TINFO, "(case%02d) END => %s",
-					 i, (ret == 0) ? "OK" : "NG");
-				result |= ret;
-			}
-
-			switch (result) {
-			case RESULT_OK:
-				tst_resm(TPASS,
-					 "clock_nanosleep call succeeded");
-				break;
-
-			default:
-				tst_brkm(TFAIL | TERRNO, cleanup,
-					 "clock_nanosleep failed");
-				break;
-			}
-
-		}
 
+	/* result check */
+	if (!TEST_RETURN && (elapsed_ms < expect_ms - MAX_MSEC_DIFF
+		|| elapsed_ms > expect_ms + MAX_MSEC_DIFF)) {
+		tst_res(TFAIL| TTERRNO, "time range check returned %ld, expected %d, expected errno %s (%d)",
+			TEST_RETURN, tc->exp_ret, tst_strerrno(tc->exp_err), tc->exp_err);
+		return;
 	}
 
-	cleanup();
-	tst_exit();
-
+	if (tc->ttype == SEND_SIGINT && !rm.tv_sec && !rm.tv_nsec) {
+		tst_res(TFAIL | TTERRNO, "remain time check returned %ld, expected %d, expected errno %s (%d)",
+			TEST_RETURN, tc->exp_ret, tst_strerrno(tc->exp_err), tc->exp_err);
+		return;
+	}
+	if (TEST_RETURN != tc->exp_ret) {
+		tst_res(TFAIL | TTERRNO, "returned %ld, expected %d, expected errno: %s (%d)",
+			TEST_RETURN, tc->exp_ret, tst_strerrno(tc->exp_err), tc->exp_err);
+		return;
+	}
+	tst_res(TPASS, "returned %ld", TEST_RETURN);
 }
+
+static struct tst_test test = {
+	.tid = "clock_nanosleep01",
+	.tcnt = ARRAY_SIZE(tcase),
+	.test = do_test,
+	.setup = setup,
+	.forks_child = 1,
+};
-- 
git-series 0.9.1

[LTP] [PATCH v3 2/2] testcases/clock_nanosleep01: convert to use new test library API

[Cyril Hrubis]

Hi!
> +	tst_res(TINFO, "remain time: %ld %ld", rm.tv_sec, rm.tv_nsec);

Looking closer this message should be printed only for the SEND_SIGINT
testcase.

> +	/* result check */
> +	if (!TEST_RETURN && (elapsed_ms < expect_ms - MAX_MSEC_DIFF
> +		|| elapsed_ms > expect_ms + MAX_MSEC_DIFF)) {
> +		tst_res(TFAIL| TTERRNO, "time range check returned %ld, expected %d, expected errno %s (%d)",
> +			TEST_RETURN, tc->exp_ret, tst_strerrno(tc->exp_err), tc->exp_err);
> +		return;
>  	}

Sorry that I missed that in the earlier patches. The actuall tst_res()
message should explain the reason for the failure here. So something as:

tst_res(TFAIL, "The clock_nanosleep() haven't slept correctly, "
        "measured %ldms, expected %ldms +- %d", elapsed_ms, expect_ms,
	MAX_MSEC_DIFF);

> -	cleanup();
> -	tst_exit();
> -
> +	if (tc->ttype == SEND_SIGINT && !rm.tv_sec && !rm.tv_nsec) {
> +		tst_res(TFAIL | TTERRNO, "remain time check returned %ld, expected %d, expected errno %s (%d)",
> +			TEST_RETURN, tc->exp_ret, tst_strerrno(tc->exp_err), tc->exp_err);

Here as well, we should note that the timespec wasn't updated with
remaining time. And we should also zero the structure before the
SEND_SIGINT test, since otherwies this will pass even if kernel does not
store the value since it will contain random data from stack.

And if we wanted to be even more pedantic we should also check that the
remaining time is <= sleep time.

> +		return;
> +	}
> +	if (TEST_RETURN != tc->exp_ret) {
> +		tst_res(TFAIL | TTERRNO, "returned %ld, expected %d, expected errno: %s (%d)",
> +			TEST_RETURN, tc->exp_ret, tst_strerrno(tc->exp_err), tc->exp_err);
> +		return;
> +	}
> +	tst_res(TPASS, "returned %ld", TEST_RETURN);
>  }

Otherwise this version looks very good. Just keep in mind to keep the
lines under 80 characters.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 2/2] testcases/clock_nanosleep01: convert to use new test library API

[Petr Vorel]

Hi Cyril,

again, thanks for your review!

<snip>
> Sorry that I missed that in the earlier patches. The actuall tst_res()
> message should explain the reason for the failure here. So something as:

> tst_res(TFAIL, "The clock_nanosleep() haven't slept correctly, "
>         "measured %ldms, expected %ldms +- %d", elapsed_ms, expect_ms,
> 	MAX_MSEC_DIFF);
This looks better :-). I consider too verbose output (return code, expected return code,
expected errno etc). useless, but didn't dare to remove it.

<snip>

> Otherwise this version looks very good. Just keep in mind to keep the
> lines under 80 characters.
Right! I'm sorry about being relaxed on this so far.
Just a question about strings: kernel formatting style [1] says about it:
> However, never break user-visible strings such as
> printk messages, because that breaks the ability to grep for them.
And indeed my using plugin forcing kernel coding style [2] doesn't complain on strings
So I hope string long up to 90 characters are still ok.

Kind regards,
Petr

[1] https://www.kernel.org/doc/Documentation/CodingStyle
[2] https://github.com/vivien/vim-linux-coding-style
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.linux.it/pipermail/ltp/attachments/20161125/818ea698/attachment.sig>

[LTP] [PATCH v3 2/2] testcases/clock_nanosleep01: convert to use new test library API

[Cyril Hrubis]

Hi!
> > Otherwise this version looks very good. Just keep in mind to keep the
> > lines under 80 characters.
> Right! I'm sorry about being relaxed on this so far.
> Just a question about strings: kernel formatting style [1] says about it:
> > However, never break user-visible strings such as
> > printk messages, because that breaks the ability to grep for them.
> And indeed my using plugin forcing kernel coding style [2] doesn't complain on strings
> So I hope string long up to 90 characters are still ok.

As far as I can tell we do not have any hard rules about this in LTP,
some code splits up the strings, some does not. I personally am trying
to keep things as short as possible while including all important bits
and I'm also avoiding unnecessary levels of indentation which keeps
things under 80 chars most of the time. I tend to split the string if
the line goes too long, which seems to be more or less about 130 chars.

-- 
Cyril Hrubis
chrubis@suse.cz