All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH 0/4] Convert tests to use new library
@ 2019-03-25 23:20 Sandeep Patil
  2019-03-25 23:20 ` [LTP] [RFC PATCH 1/4] syscalls/abort01: convert to " Sandeep Patil
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Sandeep Patil @ 2019-03-25 23:20 UTC (permalink / raw)
  To: ltp

The series of patches convert 4 tests to use the new library. I
went through the kernel/syscalls directory alphabetically and
found the tests that haven't been converted yet.

The tests I changed here are, abort01, accept01, accept4 and acct1.

I've only tested these with glibc + x86 vm. I removed some of the
UCLINUX specific ifdefs in the process that I am not so sure about.
Is there a way for me to test these on other systems?

For now, I marked the patch where I removed these as RFC.

Sandeep Patil (4):
  syscalls/abort01: convert to new library
  syscalls/accept01: convert to new library.
  syscalls/accept4: convert to new library
  syscalls/acct01: convert to new library

 testcases/kernel/syscalls/abort/abort01.c     | 171 ++++---------
 testcases/kernel/syscalls/accept/accept01.c   | 236 ++++++++---------
 .../kernel/syscalls/accept4/accept4_01.c      | 241 ++++++------------
 testcases/kernel/syscalls/acct/acct01.c       | 207 +++++----------
 4 files changed, 294 insertions(+), 561 deletions(-)

-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [LTP] [RFC PATCH 1/4] syscalls/abort01: convert to new library
  2019-03-25 23:20 [LTP] [PATCH 0/4] Convert tests to use new library Sandeep Patil
@ 2019-03-25 23:20 ` Sandeep Patil
  2019-03-26 11:58   ` Cyril Hrubis
  2019-03-25 23:20 ` [LTP] [PATCH 2/4] syscalls/accept01: " Sandeep Patil
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Sandeep Patil @ 2019-03-25 23:20 UTC (permalink / raw)
  To: ltp

From: Sandeep Patil <sspatil@google.com>

In the process, drop checks for UCLINUX and WCOREDUMP.
Make the test simple and remove all the logic to detect if a
system is "in stress".

Signed-off-by: Sandeep Patil <sspatil@android.com>
---
 testcases/kernel/syscalls/abort/abort01.c | 171 ++++++----------------
 1 file changed, 48 insertions(+), 123 deletions(-)

diff --git a/testcases/kernel/syscalls/abort/abort01.c b/testcases/kernel/syscalls/abort/abort01.c
index 3a5dff585..ac5ddb140 100644
--- a/testcases/kernel/syscalls/abort/abort01.c
+++ b/testcases/kernel/syscalls/abort/abort01.c
@@ -1,25 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
 /*
  *   Copyright (c) International Business Machines  Corp., 2002
  *   01/02/2003	Port to LTP	avenkat@us.ibm.com
  *   11/11/2002: Ported to LTP Suite by Ananda
  *   06/30/2001	Port to Linux	nsharoff@us.ibm.com
  *
- *   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
- */
-
- /* ALGORITHM
+ * ALGORITHM
  *	Fork child.  Have child abort, check return status.
  *
  * RESTRICTIONS
@@ -35,132 +22,70 @@
 #include <unistd.h>
 #include <sys/resource.h>
 
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"
 
-#define NUM 3
-
-char *TCID = "abort01";
-int TST_TOTAL = 1;
-
-static void setup(void);
-static void cleanup(void);
-static void do_child();
-static int instress();
+static void do_child(void)
+{
+	abort();
+	fprintf(stderr, "\tchild - abort failed.\n");
+	exit(1);
+}
 
-int main(int argc, char *argv[])
+void verify_abort(unsigned int nr)
 {
-	register int i;
-	int status, count, child, kidpid;
+	int i;
+	int status, child, kidpid;
 	int sig, ex;
-
-#ifdef WCOREDUMP
 	int core;
-	core = 0;
-#endif
-	ex = sig = 0;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-#ifdef UCLINUX
-	maybe_run_child(&do_child, "");
-#endif
-
-	setup();
-
-	for (i = 0; i < NUM; i++) {
-		kidpid = FORK_OR_VFORK();
-		if (kidpid == 0) {
-#ifdef UCLINUX
-			if (self_exec(argv[0], "")) {
-				if (!instress()) {
-					perror("fork failed");
-					exit(1);
-				}
-			}
-#else
-			do_child();
-#endif
-		}
-		if (kidpid < 0)
-			if (!instress())
-				tst_brkm(TBROK | TERRNO, cleanup,
-					 "fork failed");
-		count = 0;
-		while ((child = wait(&status)) > 0)
-			count++;
-		if (count != 1) {
-			tst_brkm(TBROK, cleanup,
-				 "wrong # children waited on; got %d, expected 1",
-				 count);
-		}
-		if (WIFSIGNALED(status)) {
+	core = ex = sig = 0;
 
-#ifdef WCOREDUMP
-			core = WCOREDUMP(status);
-#endif
-			sig = WTERMSIG(status);
+	kidpid = SAFE_FORK();
+	if (kidpid == 0)
+		do_child();
 
-		}
-		if (WIFEXITED(status))
-			ex = WEXITSTATUS(status);
+	child = SAFE_WAIT(&status);
 
-#ifdef WCOREDUMP
-		if (core == 0) {
-			tst_brkm(TFAIL, cleanup,
-				 "Child did not dump core; exit code = %d, "
-				 "signal = %d", ex, sig);
-		} else if (core != -1) {
-			tst_resm(TPASS, "abort dumped core");
-		}
-#endif
-		if (sig == SIGIOT) {
-			tst_resm(TPASS, "abort raised SIGIOT");
-		} else {
-			tst_brkm(TFAIL, cleanup,
-				 "Child did not raise SIGIOT (%d); exit code = %d, "
-				 "signal = %d", SIGIOT, ex, sig);
-		}
+	if (WIFSIGNALED(status)) {
+		core = WCOREDUMP(status);
+		sig = WTERMSIG(status);
 
 	}
 
-	cleanup();
-	tst_exit();
+	if (WIFEXITED(status))
+		ex = WEXITSTATUS(status);
+
+	if (core == 0)
+		tst_brk(TFAIL,
+			"Missing core dump; exit(%d), signal(%d)",
+			ex, sig);
+	else if (core != -1)
+		tst_res(TPASS, "abort() dumped core");
+
+	if (sig == SIGIOT)
+		tst_res(TPASS, "abort() raised SIGIOT");
+	else
+		tst_brk(TFAIL,
+			"Unexpected signal(%d), expected SIGIOT(%d)",
+			sig, SIGIOT);
 }
 
-/* 1024 GNU blocks */
-#define MIN_RLIMIT_CORE (1024 * 1024)
-
 static void setup(void)
 {
+#define MIN_RLIMIT_CORE (1024 * 1024)
 	struct rlimit rlim;
 
-	SAFE_GETRLIMIT(NULL, RLIMIT_CORE, &rlim);
-
+	/* make sure we get core dumps */
+	SAFE_GETRLIMIT(RLIMIT_CORE, &rlim);
 	if (rlim.rlim_cur < MIN_RLIMIT_CORE) {
-		tst_resm(TINFO, "Adjusting RLIMIT_CORE to %i", MIN_RLIMIT_CORE);
 		rlim.rlim_cur = MIN_RLIMIT_CORE;
-		SAFE_SETRLIMIT(NULL, RLIMIT_CORE, &rlim);
+		SAFE_SETRLIMIT(RLIMIT_CORE, &rlim);
 	}
-
-	tst_tmpdir();
 }
 
-static void cleanup(void)
-{
-	unlink("core");
-	tst_rmdir();
-}
-
-static void do_child(void)
-{
-	abort();
-	fprintf(stderr, "\tchild - abort failed.\n");
-	exit(1);
-}
-
-static int instress(void)
-{
-	tst_resm(TINFO,
-		 "System resources may be too low; fork(), select() etc are likely to fail.");
-	return 1;
-}
+static struct tst_test test = {
+	.tcnt = 3,
+	.needs_tmpdir = 1,
+	.forks_child = 1,
+	.setup = setup,
+	.test = verify_abort,
+};
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [LTP] [PATCH 2/4] syscalls/accept01: convert to new library.
  2019-03-25 23:20 [LTP] [PATCH 0/4] Convert tests to use new library Sandeep Patil
  2019-03-25 23:20 ` [LTP] [RFC PATCH 1/4] syscalls/abort01: convert to " Sandeep Patil
@ 2019-03-25 23:20 ` Sandeep Patil
  2019-03-26 12:35   ` Cyril Hrubis
  2019-03-25 23:20 ` [LTP] [PATCH 3/4] syscalls/accept4: " Sandeep Patil
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Sandeep Patil @ 2019-03-25 23:20 UTC (permalink / raw)
  To: ltp

...and drop the duplicate "invalid addrlen" test case.

Signed-off-by: Sandeep Patil <sspatil@android.com>
---
 testcases/kernel/syscalls/accept/accept01.c | 236 +++++++++-----------
 1 file changed, 103 insertions(+), 133 deletions(-)

diff --git a/testcases/kernel/syscalls/accept/accept01.c b/testcases/kernel/syscalls/accept/accept01.c
index b50056520..5982c09f8 100644
--- a/testcases/kernel/syscalls/accept/accept01.c
+++ b/testcases/kernel/syscalls/accept/accept01.c
@@ -1,26 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
 /*
- *
  *   Copyright (c) International Business Machines  Corp., 2001
  *   07/2001 Ported by Wayne Boyer
  *
- *   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
- */
-
-/*
- * Description:
- *  Verify that accept() returns the proper errno for various failure cases
+ *   Description:
+ *     Verify that accept() returns the proper errno for various failure cases
  */
 
 #include <stdio.h>
@@ -34,150 +19,135 @@
 
 #include <netinet/in.h>
 
-#include "test.h"
-#include "safe_macros.h"
-
-char *TCID = "accept01";
-int testno;
+#include "tst_test.h"
 
-int s;				/* socket descriptor */
 struct sockaddr_in sin0, fsin1;
-socklen_t sinlen;
-
-static void setup(void);
-static void cleanup(void);
-static void setup0(void);
-static void cleanup0(void);
-static void setup1(void);
-static void cleanup1(void);
-static void setup2(void);
-static void setup3(void);
-
-struct test_case_t {		/* test case structure */
+
+struct test_case;
+static void setup_invalid_fd(struct test_case *tcase);
+static void setup_nonsocket_fd(struct test_case *tcase);
+static void setup_socket(struct test_case *tcase);
+static void setup_fionbio(struct test_case *tcase);
+static void cleanup_tcase(struct test_case *tcase);
+
+/* test cases */
+static struct test_case {
 	int domain;		/* PF_INET, PF_UNIX, ... */
 	int type;		/* SOCK_STREAM, SOCK_DGRAM ... */
 	int proto;		/* protocol number (usually 0 = default) */
+	int socketfd;		/* socketfd for the test case */
 	struct sockaddr *sockaddr;	/* socket address buffer */
-	socklen_t *salen;	/* accept's 3rd argument */
+	socklen_t salen;	/* accept's 3rd argument */
 	int retval;		/* syscall return value */
 	int experrno;		/* expected errno */
-	void (*setup) (void);
-	void (*cleanup) (void);
+	void (*setup) (struct test_case *);
+	void (*cleanup) (struct test_case *);
 	char *desc;
-} tdat[] = {
+} tcases[] = {
 	{
-	PF_INET, SOCK_STREAM, 0, (struct sockaddr *)&fsin1,
-		    &sinlen, -1, EBADF, setup0, cleanup0,
-		    "bad file descriptor"}, {
-	PF_INET, SOCK_STREAM, 0, (struct sockaddr *)&fsin1,
-		    &sinlen, -1, ENOTSOCK, setup0, cleanup0,
-		    "bad file descriptor"}, {
-	PF_INET, SOCK_STREAM, 0, (struct sockaddr *)3,
-		    &sinlen, -1, EINVAL, setup1, cleanup1,
-		    "invalid socket buffer"}, {
-	PF_INET, SOCK_STREAM, 0, (struct sockaddr *)&fsin1,
-		    (socklen_t *) 1, -1, EINVAL, setup1, cleanup1,
-		    "invalid salen"}, {
-	PF_INET, SOCK_STREAM, 0, (struct sockaddr *)&fsin1,
-		    &sinlen, -1, EINVAL, setup2, cleanup1,
-		    "invalid salen"}, {
-	PF_INET, SOCK_STREAM, 0, (struct sockaddr *)&fsin1,
-		    &sinlen, -1, EINVAL, setup3, cleanup1,
-		    "no queued connections"}, {
-	PF_INET, SOCK_DGRAM, 0, (struct sockaddr *)&fsin1,
-		    &sinlen, -1, EOPNOTSUPP, setup1, cleanup1,
-		    "UDP accept"},};
-
-int TST_TOTAL = sizeof(tdat) / sizeof(tdat[0]);
-
-int main(int ac, char *av[])
-{
-	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) {
-			tdat[testno].setup();
-
-			TEST(accept(s, tdat[testno].sockaddr,
-				    tdat[testno].salen));
-			if (TEST_RETURN > 0)
-				TEST_RETURN = 0;
-			if (TEST_RETURN != tdat[testno].retval ||
-			    (TEST_RETURN < 0 &&
-			     TEST_ERRNO != tdat[testno].experrno)) {
-				tst_resm(TFAIL, "%s ; returned"
-					 " %ld (expected %d), errno %d (expected"
-					 " %d)", tdat[testno].desc,
-					 TEST_RETURN, tdat[testno].retval,
-					 TEST_ERRNO, tdat[testno].experrno);
-			} else {
-				tst_resm(TPASS, "%s successful",
-					 tdat[testno].desc);
-			}
-			tdat[testno].cleanup();
-		}
-	}
-
-	cleanup();
-	tst_exit();
-}
+		PF_INET, SOCK_STREAM, 0, -1, (struct sockaddr *)&fsin1,
+		sizeof(fsin1), -1, EBADF, setup_invalid_fd, NULL,
+		"bad file descriptor"
+	},
+	{
+		PF_INET, SOCK_STREAM, 0, -1, (struct sockaddr *)&fsin1,
+		sizeof(fsin1), -1, ENOTSOCK, setup_nonsocket_fd,
+		cleanup_tcase, "bad file descriptor"
+	},
+	{
+		PF_INET, SOCK_STREAM, 0, -1, (struct sockaddr *)3,
+		sizeof(fsin1), -1, EINVAL, setup_socket, cleanup_tcase,
+		"invalid socket buffer"
+	},
+	{
+		PF_INET, SOCK_STREAM, 0, -1, (struct sockaddr *)&fsin1,
+		1, -1, EINVAL, setup_socket, cleanup_tcase,
+		"invalid salen"
+	},
+	{
+		PF_INET, SOCK_STREAM, 0, -1, (struct sockaddr *)&fsin1,
+		sizeof(fsin1), -1, EINVAL, setup_fionbio, cleanup_tcase,
+		"no queued connections"
+	},
+	{
+		PF_INET, SOCK_DGRAM, 0, -1, (struct sockaddr *)&fsin1,
+		sizeof(fsin1), -1, EOPNOTSUPP, setup_socket, cleanup_tcase,
+		"UDP accept"
+	},
+};
 
-static void setup(void)
-{
-	TEST_PAUSE;
 
-	/* initialize local sockaddr */
-	sin0.sin_family = AF_INET;
-	sin0.sin_port = 0;
-	sin0.sin_addr.s_addr = INADDR_ANY;
-}
 
-static void cleanup(void)
+static void setup_invalid_fd(struct test_case *tcase)
 {
+	tcase->socketfd = 400; /* anything that is not an open file */
 }
 
-static void setup0(void)
+static void setup_nonsocket_fd(struct test_case *tcase)
 {
-	if (tdat[testno].experrno == EBADF)
-		s = 400;	/* anything not an open file */
-	else if ((s = open("/dev/null", O_WRONLY)) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "error opening /dev/null");
+	tcase->socketfd = SAFE_OPEN("/dev/null", O_WRONLY);
 }
 
-static void cleanup0(void)
+static void setup_socket(struct test_case *tcase)
 {
-	s = -1;
+	tcase->socketfd = SAFE_SOCKET(tcase->domain, tcase->type, tcase->proto);
+	SAFE_BIND(tcase->socketfd, (struct sockaddr *)&sin0, sizeof(sin0));
+	tcase->salen = sizeof(fsin1);
 }
 
-static void setup1(void)
+static void setup_fionbio(struct test_case *tcase)
 {
-	s = SAFE_SOCKET(cleanup, tdat[testno].domain, tdat[testno].type,
-			tdat[testno].proto);
-	SAFE_BIND(cleanup, s, (struct sockaddr *)&sin0, sizeof(sin0));
-	sinlen = sizeof(fsin1);
+	int one = 1;
+	setup_socket(tcase);
+	SAFE_IOCTL(tcase->socketfd, FIONBIO, &one);
 }
 
-static void cleanup1(void)
+static void cleanup_tcase(struct test_case *tcase)
 {
-	(void)close(s);
-	s = -1;
+	if (tcase->socketfd >= 0)
+		SAFE_CLOSE(tcase->socketfd);
+
+	tcase->socketfd = -1;
 }
 
-static void setup2(void)
+void verify_accept(unsigned int nr)
 {
-	setup1();		/* get a socket in s */
-	sinlen = 1;		/* invalid s */
+	struct test_case *tcase = &tcases[nr];
+
+	if (tcase->setup)
+		tcase->setup(tcase);
+
+	TEST(accept(tcase->socketfd, tcase->sockaddr, &tcase->salen));
+	if (TST_RET > 0) {
+		/* don't leak accepted socket, close them first */
+		SAFE_CLOSE(TST_RET);
+		TST_RET = 0;
+	}
+
+	if (TST_RET != tcase->retval ||
+		(TST_RET < 0 && TST_ERR != tcase->experrno)) {
+		tst_res(TFAIL, "%s ; returned"
+				" %ld (expected %d), errno %d (expected"
+				" %d)", tcase->desc, TST_RET, tcase->retval,
+				TST_ERR, tcase->experrno);
+	} else {
+		tst_res(TPASS, "%s successful", tcase->desc);
+	}
+
+	if (tcase->cleanup)
+		tcase->cleanup(tcase);
 }
 
-static void setup3(void)
+static void test_setup(void)
 {
-	int one = 1;
-
-	setup1();
-	SAFE_IOCTL(cleanup, s, FIONBIO, &one);
+	/* initialize local sockaddr */
+	sin0.sin_family = AF_INET;
+	sin0.sin_port = 0;
+	sin0.sin_addr.s_addr = INADDR_ANY;
 }
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.setup = test_setup,
+	.test = verify_accept,
+};
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [LTP] [PATCH 3/4] syscalls/accept4: convert to new library
  2019-03-25 23:20 [LTP] [PATCH 0/4] Convert tests to use new library Sandeep Patil
  2019-03-25 23:20 ` [LTP] [RFC PATCH 1/4] syscalls/abort01: convert to " Sandeep Patil
  2019-03-25 23:20 ` [LTP] [PATCH 2/4] syscalls/accept01: " Sandeep Patil
@ 2019-03-25 23:20 ` Sandeep Patil
  2019-03-26 15:33   ` Cyril Hrubis
  2019-03-25 23:20 ` [LTP] [PATCH 4/4] syscalls/acct01: " Sandeep Patil
  2019-03-26  9:42 ` [LTP] [PATCH 0/4] Convert tests to use " Cyril Hrubis
  4 siblings, 1 reply; 17+ messages in thread
From: Sandeep Patil @ 2019-03-25 23:20 UTC (permalink / raw)
  To: ltp

Remove all debug checks and simplify the test.

Signed-off-by: Sandeep Patil <sspatil@android.com>
---
 .../kernel/syscalls/accept4/accept4_01.c      | 241 ++++++------------
 1 file changed, 81 insertions(+), 160 deletions(-)

diff --git a/testcases/kernel/syscalls/accept4/accept4_01.c b/testcases/kernel/syscalls/accept4/accept4_01.c
index dec4ef93b..036457571 100644
--- a/testcases/kernel/syscalls/accept4/accept4_01.c
+++ b/testcases/kernel/syscalls/accept4/accept4_01.c
@@ -1,23 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
 /*
- *
  * Copyright (C) 2008, Linux Foundation,
  * written by Michael Kerrisk <mtk.manpages@gmail.com>
  * Initial Porting to LTP by Subrata <subrata@linux.vnet.ibm.com>
  *
- * Licensed under the GNU GPLv2 or later.
- * 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
  */
 
 #define _GNU_SOURCE
@@ -31,14 +18,12 @@
 #include <string.h>
 #include <errno.h>
 
-#include "test.h"
+#include "tst_test.h"
 #include "lapi/fcntl.h"
 #include "lapi/syscalls.h"
 
 #define PORT_NUM 33333
 
-#define die(msg)	tst_brkm(TBROK|TERRNO, cleanup, msg)
-
 #ifndef SOCK_CLOEXEC
 #define SOCK_CLOEXEC    O_CLOEXEC
 #endif
@@ -50,39 +35,13 @@
 #define USE_SOCKETCALL 1
 #endif
 
-char *TCID = "accept04_01";
-int TST_TOTAL = 1;
-
-static void setup(void)
-{
-	TEST_PAUSE;
-	tst_tmpdir();
-}
-
-static void cleanup(void)
-{
-	tst_rmdir();
-}
+struct sockaddr_in conn_addr;
+int listening_fd;
 
 #if !(__GLIBC_PREREQ(2, 10))
 static int
 accept4_01(int fd, struct sockaddr *sockaddr, socklen_t *addrlen, int flags)
 {
-#ifdef DEBUG
-	tst_resm(TINFO, "Calling accept4(): flags = %x", flags);
-	if (flags != 0) {
-		tst_resm(TINFO, " (");
-		if (flags & SOCK_CLOEXEC)
-			tst_resm(TINFO, "SOCK_CLOEXEC");
-		if ((flags & SOCK_CLOEXEC) && (flags & SOCK_NONBLOCK))
-			tst_resm(TINFO, " ");
-		if (flags & SOCK_NONBLOCK)
-			tst_resm(TINFO, "SOCK_NONBLOCK");
-		tst_resm(TINFO, ")");
-	}
-	tst_resm(TINFO, "\n");
-#endif
-
 #if USE_SOCKETCALL
 	long args[6];
 
@@ -98,79 +57,7 @@ accept4_01(int fd, struct sockaddr *sockaddr, socklen_t *addrlen, int flags)
 }
 #endif
 
-static void
-do_test(int lfd, struct sockaddr_in *conn_addr,
-	int closeonexec_flag, int nonblock_flag)
-{
-	int connfd, acceptfd;
-	int fdf, flf, fdf_pass, flf_pass;
-	struct sockaddr_in claddr;
-	socklen_t addrlen;
-
-#ifdef DEBUG
-	tst_resm(TINFO, "=======================================\n");
-#endif
-
-	connfd = socket(AF_INET, SOCK_STREAM, 0);
-	if (connfd == -1)
-		die("Socket Error");
-	if (connect(connfd, (struct sockaddr *)conn_addr,
-		    sizeof(struct sockaddr_in)) == -1)
-		die("Connect Error");
-
-	addrlen = sizeof(struct sockaddr_in);
-#if !(__GLIBC_PREREQ(2, 10))
-	acceptfd = accept4_01(lfd, (struct sockaddr *)&claddr, &addrlen,
-			      closeonexec_flag | nonblock_flag);
-#else
-	acceptfd = accept4(lfd, (struct sockaddr *)&claddr, &addrlen,
-			   closeonexec_flag | nonblock_flag);
-#endif
-	if (acceptfd == -1) {
-		if (errno == ENOSYS) {
-			tst_brkm(TCONF, cleanup,
-			         "syscall __NR_accept4 not supported");
-		} else {
-			tst_brkm(TBROK | TERRNO, cleanup, "accept4 failed");
-		}
-	}
-
-	fdf = fcntl(acceptfd, F_GETFD);
-	if (fdf == -1)
-		die("fcntl:F_GETFD");
-	fdf_pass = ((fdf & FD_CLOEXEC) != 0) ==
-	    ((closeonexec_flag & SOCK_CLOEXEC) != 0);
-#ifdef DEBUG
-	tst_resm(TINFO, "Close-on-exec flag is %sset (%s); ",
-		 (fdf & FD_CLOEXEC) ? "" : "not ", fdf_pass ? "OK" : "failed");
-#endif
-	if (!fdf_pass)
-		tst_resm(TFAIL,
-			 "Close-on-exec flag mismatch, should be %x, actual %x",
-			 fdf & FD_CLOEXEC, closeonexec_flag & SOCK_CLOEXEC);
-
-	flf = fcntl(acceptfd, F_GETFL);
-	if (flf == -1)
-		die("fcntl:F_GETFD");
-	flf_pass = ((flf & O_NONBLOCK) != 0) ==
-	    ((nonblock_flag & SOCK_NONBLOCK) != 0);
-#ifdef DEBUG
-	tst_resm(TINFO, "nonblock flag is %sset (%s)\n",
-		 (flf & O_NONBLOCK) ? "" : "not ", flf_pass ? "OK" : "failed");
-#endif
-	if (!flf_pass)
-		tst_resm(TFAIL,
-			 "nonblock flag mismatch, should be %x, actual %x",
-			 fdf & O_NONBLOCK, nonblock_flag & SOCK_NONBLOCK);
-
-	close(acceptfd);
-	close(connfd);
-
-	if (fdf_pass && flf_pass)
-		tst_resm(TPASS, "Test passed");
-}
-
-static int create_listening_socket(int port_num)
+static int create_listening_socket(void)
 {
 	struct sockaddr_in svaddr;
 	int lfd;
@@ -179,65 +66,99 @@ static int create_listening_socket(int port_num)
 	memset(&svaddr, 0, sizeof(struct sockaddr_in));
 	svaddr.sin_family = AF_INET;
 	svaddr.sin_addr.s_addr = htonl(INADDR_ANY);
-	svaddr.sin_port = htons(port_num);
+	svaddr.sin_port = htons(PORT_NUM);
 
-	lfd = socket(AF_INET, SOCK_STREAM, 0);
-	if (lfd == -1)
-		die("Socket Error");
+	lfd = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0);
 
 	optval = 1;
-	if (setsockopt(lfd, SOL_SOCKET, SO_REUSEADDR, &optval,
-		       sizeof(optval)) == -1)
-		die("Setsockopt Error");
-
-	if (bind(lfd, (struct sockaddr *)&svaddr,
-		 sizeof(struct sockaddr_in)) == -1)
-		die("Bind Error");
-
-	if (listen(lfd, 5) == -1)
-		die("Listen Error");
+	SAFE_SETSOCKOPT(lfd, SOL_SOCKET, SO_REUSEADDR, &optval, sizeof(optval));
+	SAFE_BIND(lfd, (struct sockaddr *)&svaddr, sizeof(struct sockaddr_in));
+	SAFE_LISTEN(lfd, 5);
 
 	return lfd;
 }
 
-static char *opt_port;
+void setup(void)
+{
+	memset(&conn_addr, 0, sizeof(struct sockaddr_in));
+	conn_addr.sin_family = AF_INET;
+	conn_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+	conn_addr.sin_port = htons(PORT_NUM);
 
-static option_t options[] = {
-	{"p:", NULL, &opt_port},
-	{NULL, NULL, NULL}
-};
+	listening_fd = create_listening_socket();
+}
 
-static void usage(void)
+void cleanup(void)
 {
-	printf("  -p      Port\n");
+	SAFE_CLOSE(listening_fd);
 }
 
-int main(int argc, char *argv[])
+static struct test_case {
+	int cloexec;
+	int nonblock;
+} tcases[] = {
+	{ 0, 0 },
+	{ SOCK_CLOEXEC, 0 },
+	{ 0, SOCK_NONBLOCK },
+	{ SOCK_CLOEXEC, SOCK_NONBLOCK },
+};
+
+void verify_accept4(unsigned int nr)
 {
-	struct sockaddr_in conn_addr;
-	int lfd;
-	int port_num = PORT_NUM;
+	struct test_case *tcase = &tcases[nr];
+	int connfd, acceptfd;
+	int fdf, flf, fdf_pass, flf_pass;
+	struct sockaddr_in claddr;
+	socklen_t addrlen;
 
-	tst_parse_opts(argc, argv, options, usage);
+	connfd = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0);
+	SAFE_CONNECT(connfd, (struct sockaddr *)&conn_addr, sizeof(conn_addr));
+	addrlen = sizeof(claddr);
 
-	if (opt_port)
-		port_num = atoi(opt_port);
+#if !(__GLIBC_PREREQ(2, 10))
+	TEST(accept4_01(listening_fd, (struct sockaddr *)&claddr, &addrlen,
+				tcase->cloexec | tcase->nonblock));
+#else
+	TEST(accept4(listening_fd, (struct sockaddr *)&claddr, &addrlen,
+				tcase->cloexec | tcase->nonblock));
+#endif
+	if (TST_RET == -1) {
+		if (TST_ERR == ENOSYS)
+			tst_brk(TCONF, "syscall __NR_accept4 not supported");
+		else
+			tst_brk(TBROK | TTERRNO, "accept4 failed");
+	}
 
-	setup();
+	acceptfd = TST_RET;
 
-	memset(&conn_addr, 0, sizeof(struct sockaddr_in));
-	conn_addr.sin_family = AF_INET;
-	conn_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
-	conn_addr.sin_port = htons(port_num);
+	/* Test to see if O_CLOEXEC is as expected */
+	fdf = SAFE_FCNTL(acceptfd, F_GETFD);
+	fdf_pass = !!(fdf & FD_CLOEXEC) == !!(tcase->cloexec & SOCK_CLOEXEC);
+	if (!fdf_pass)
+		tst_res(TFAIL,  "Close-on-exec flag mismatch, %d vs %d",
+			 !!(fdf & FD_CLOEXEC),
+			 !!(tcase->cloexec & SOCK_CLOEXEC));
+
+	/* Test to see if O_NONBLOCK is as expected */
+	flf = SAFE_FCNTL(acceptfd, F_GETFL);
+	flf_pass = !!(flf & O_NONBLOCK) == !!(tcase->nonblock & SOCK_NONBLOCK);
+	if (!flf_pass)
+		tst_res(TFAIL, "nonblock flag mismatch, %d vs %d",
+			 !!(fdf & O_NONBLOCK),
+			 !!(tcase->nonblock & SOCK_NONBLOCK));
 
-	lfd = create_listening_socket(port_num);
+	SAFE_CLOSE(acceptfd);
+	SAFE_CLOSE(connfd);
 
-	do_test(lfd, &conn_addr, 0, 0);
-	do_test(lfd, &conn_addr, SOCK_CLOEXEC, 0);
-	do_test(lfd, &conn_addr, 0, SOCK_NONBLOCK);
-	do_test(lfd, &conn_addr, SOCK_CLOEXEC, SOCK_NONBLOCK);
+	if (fdf_pass && flf_pass)
+		tst_res(TPASS, "Test passed: Close-on-exec %d, nonblock %d",
+				fdf_pass, flf_pass);
 
-	close(lfd);
-	cleanup();
-	tst_exit();
 }
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.setup = setup,
+	.cleanup = cleanup,
+	.test = verify_accept4,
+};
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [LTP] [PATCH 4/4] syscalls/acct01: convert to new library
  2019-03-25 23:20 [LTP] [PATCH 0/4] Convert tests to use new library Sandeep Patil
                   ` (2 preceding siblings ...)
  2019-03-25 23:20 ` [LTP] [PATCH 3/4] syscalls/accept4: " Sandeep Patil
@ 2019-03-25 23:20 ` Sandeep Patil
  2019-03-26 16:11   ` Cyril Hrubis
  2019-03-26  9:42 ` [LTP] [PATCH 0/4] Convert tests to use " Cyril Hrubis
  4 siblings, 1 reply; 17+ messages in thread
From: Sandeep Patil @ 2019-03-25 23:20 UTC (permalink / raw)
  To: ltp

Use pre-created files provided by the library instead of creating
them by hand in the setup() routine.

Signed-off-by: Sandeep Patil <sspatil@android.com>
---
 testcases/kernel/syscalls/acct/acct01.c | 207 +++++++-----------------
 1 file changed, 62 insertions(+), 145 deletions(-)

diff --git a/testcases/kernel/syscalls/acct/acct01.c b/testcases/kernel/syscalls/acct/acct01.c
index eb104315b..b2bde13ed 100644
--- a/testcases/kernel/syscalls/acct/acct01.c
+++ b/testcases/kernel/syscalls/acct/acct01.c
@@ -1,31 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
 /*
- *
  *  Copyright (c) International Business Machines  Corp., 2002
- *
- *  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
  */
 
 /* 12/03/2002	Port to LTP     robbiew@us.ibm.com */
 /* 06/30/2001	Port to Linux	nsharoff@us.ibm.com */
 
-/*
- * ALGORITHM
- *	issue calls to acct and test the returned values against
- *	expected results
- */
-
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <errno.h>
@@ -36,8 +17,7 @@
 #include <unistd.h>
 #include <sys/mount.h>
 
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"
 
 #define DIR_MODE	(S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP| \
 			 S_IXGRP|S_IROTH|S_IXOTH)
@@ -48,169 +28,106 @@
 #define TEST_FILE5	"./tmpfile"
 #define TEST_FILE6	"test_file_eloop1"
 #define TEST_FILE7	nametoolong
-#define TEST_FILE8	"mntpoint/tmp"
+#define TEST_FILE8	"mntpoint/file"
 
 static char nametoolong[PATH_MAX+2];
-static const char *fs_type;
-static const char *device;
-static int mount_flag;
+static struct passwd *ltpuser;
 
-static void setup(void);
-static void cleanup(void);
-static void setup2(void);
-static void cleanup2(void);
-static void acct_verify(int);
+static void setup_euid(void)
+{
+	SAFE_SETEUID(ltpuser->pw_uid);
+}
 
-static struct test_case_t {
+static void cleanup_euid(void)
+{
+	SAFE_SETEUID(0);
+}
+
+static struct test_case {
 	char *filename;
 	char *exp_errval;
 	int exp_errno;
 	void (*setupfunc) ();
 	void (*cleanfunc) ();
-} test_cases[] = {
+} tcases[] = {
 	{TEST_FILE1, "EISDIR",  EISDIR,  NULL,   NULL},
 	{TEST_FILE2, "EACCES",  EACCES,  NULL,   NULL},
 	{TEST_FILE3, "ENOENT",  ENOENT,  NULL,   NULL},
 	{TEST_FILE4, "ENOTDIR", ENOTDIR, NULL,   NULL},
-	{TEST_FILE5, "EPERM",   EPERM,   setup2, cleanup2},
-	{NULL,       "EPERM",   EPERM,   setup2, cleanup2},
+	{TEST_FILE5, "EPERM",   EPERM,   setup_euid, cleanup_euid},
+	{NULL,       "EPERM",   EPERM,   setup_euid, cleanup_euid},
 	{TEST_FILE6, "ELOOP",        ELOOP,        NULL, NULL},
 	{TEST_FILE7, "ENAMETOOLONG", ENAMETOOLONG, NULL, NULL},
 	{TEST_FILE8, "EROFS",        EROFS,        NULL, NULL},
 };
 
-char *TCID = "acct01";
-int TST_TOTAL = ARRAY_SIZE(test_cases);
-static struct passwd *ltpuser;
-
-int main(int argc, char *argv[])
-{
-	int lc;
-	int i;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-		for (i = 0; i < TST_TOTAL; i++)
-			acct_verify(i);
-	}
-
-	cleanup();
-	tst_exit();
-}
-
-static void check_acct_in_kernel(void)
-{
-	/* check if acct is implemented in kernel */
-	if (acct(NULL) == -1) {
-		if (errno == ENOSYS) {
-			tst_brkm(TCONF,
-				 NULL, "BSD process accounting is not configured in "
-				 "this kernel");
-		}
-	}
-}
-
 static void setup(void)
 {
 	int fd;
 
-	tst_require_root();
-
-	check_acct_in_kernel();
-
-	tst_tmpdir();
+	TEST(acct(NULL));
+	if (TST_RET == -1 && TST_ERR == ENOSYS)
+		tst_brk(TCONF, "acct() system call isn't configured in kernel");
 
-	ltpuser = SAFE_GETPWNAM(cleanup, "nobody");
+	ltpuser = SAFE_GETPWNAM("nobody");
 
-	fd = SAFE_CREAT(cleanup, TEST_FILE5, 0777);
-	SAFE_CLOSE(cleanup, fd);
+	fd = SAFE_CREAT(TEST_FILE5, 0777);
+	SAFE_CLOSE(fd);
 
-	if (acct(TEST_FILE5) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "acct failed unexpectedly");
+	TEST(acct(TEST_FILE5));
+	if (TST_RET == -1)
+		tst_brk(TBROK | TTERRNO, "acct failed unexpectedly");
 
 	/* turn off acct, so we are in a known state */
-	if (acct(NULL) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "acct(NULL) failed");
+	TEST(acct(NULL));
+	if (TST_RET == -1)
+		tst_brk(TBROK | TTERRNO, "acct(NULL) failed");
 
 	/* ELOOP SETTING */
-	SAFE_SYMLINK(cleanup, TEST_FILE6, "test_file_eloop2");
-	SAFE_SYMLINK(cleanup, "test_file_eloop2", TEST_FILE6);
+	SAFE_SYMLINK(TEST_FILE6, "test_file_eloop2");
+	SAFE_SYMLINK("test_file_eloop2", TEST_FILE6);
 
 	/* ENAMETOOLONG SETTING */
 	memset(nametoolong, 'a', PATH_MAX+1);
-
-	/* EROFS SETTING */
-	fs_type = tst_dev_fs_type();
-	device = tst_acquire_device(cleanup);
-
-	if (!device)
-		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
-
-	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
-	SAFE_MKDIR(cleanup, "mntpoint", DIR_MODE);
-	SAFE_MOUNT(cleanup, device, "mntpoint", fs_type, 0, NULL);
-	mount_flag = 1;
-
-	/* Create a file in the file system, then remount it as read-only */
-	fd = SAFE_CREAT(cleanup, TEST_FILE8, 0644);
-	SAFE_CLOSE(cleanup, fd);
-	SAFE_MOUNT(cleanup, device, "mntpoint", fs_type,
-		   MS_REMOUNT | MS_RDONLY, NULL);
 }
 
-static void acct_verify(int i)
+static void cleanup(void)
 {
-
-	if (test_cases[i].setupfunc)
-		test_cases[i].setupfunc();
-
-	TEST(acct(test_cases[i].filename));
-
-	if (test_cases[i].cleanfunc)
-		test_cases[i].cleanfunc();
-
-	if (TEST_RETURN != -1) {
-		tst_resm(TFAIL, "acct(%s) succeeded unexpectedly",
-			 test_cases[i].filename);
-		return;
-	}
-
-	if (TEST_ERRNO == test_cases[i].exp_errno) {
-		tst_resm(TPASS | TTERRNO, "acct failed as expected");
-	} else {
-		tst_resm(TFAIL | TTERRNO,
-			 "acct failed unexpectedly; expected: %d - %s",
-			 test_cases[i].exp_errno,
-			 strerror(test_cases[i].exp_errno));
-	}
+	SAFE_UNLINK(TEST_FILE5);
+	SAFE_UNLINK(TEST_FILE6);
 }
 
-static void setup2(void)
+static void verify_acct(unsigned int nr)
 {
-	SAFE_SETEUID(cleanup, ltpuser->pw_uid);
-}
+	struct test_case *tcase = &tcases[nr];
 
-static void cleanup2(void)
-{
-	SAFE_SETEUID(cleanup, 0);
-}
+	if (tcase->setupfunc)
+		tcase->setupfunc();
 
-static void cleanup(void)
-{
-	if (acct(NULL) == -1)
-		tst_resm(TWARN | TERRNO, "acct(NULL) failed");
+	TEST(acct(tcase->filename));
 
-	if (mount_flag && tst_umount("mntpoint") < 0) {
-		tst_resm(TWARN | TERRNO,
-			 "umount device:%s failed", device);
-	}
+	if (tcase->cleanfunc)
+		tcase->cleanfunc();
 
-	if (device)
-		tst_release_device(device);
+	if (TST_RET != -1)
+		tst_res(TFAIL, "acct(%s) succeeded unexpectedly",
+				tcase->filename);
 
-	tst_rmdir();
+	if (TST_ERR == tcase->exp_errno)
+		tst_res(TPASS | TTERRNO, "acct() failed as expected");
+	else
+		tst_res(TFAIL | TTERRNO,
+				"acct() failed, expected: %d - %s",
+				tcase->exp_errno, strerror(tcase->exp_errno));
 }
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+	.mntpoint = "mntpoint",
+	.needs_rofs = 1,
+	.tcnt = ARRAY_SIZE(tcases),
+	.setup = setup,
+	.cleanup = cleanup,
+	.test = verify_acct,
+};
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [LTP] [PATCH 0/4] Convert tests to use new library
  2019-03-25 23:20 [LTP] [PATCH 0/4] Convert tests to use new library Sandeep Patil
                   ` (3 preceding siblings ...)
  2019-03-25 23:20 ` [LTP] [PATCH 4/4] syscalls/acct01: " Sandeep Patil
@ 2019-03-26  9:42 ` Cyril Hrubis
  2019-03-26 12:48   ` Sandeep Patil
  4 siblings, 1 reply; 17+ messages in thread
From: Cyril Hrubis @ 2019-03-26  9:42 UTC (permalink / raw)
  To: ltp

Hi!
> I've only tested these with glibc + x86 vm. I removed some of the
> UCLINUX specific ifdefs in the process that I am not so sure about.
> Is there a way for me to test these on other systems?

The uClinux support has been deprecated and we are slowly removing the
support. The uClinux project itself seems to be dead so it does not make
sense to keep supporting it.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [LTP] [RFC PATCH 1/4] syscalls/abort01: convert to new library
  2019-03-25 23:20 ` [LTP] [RFC PATCH 1/4] syscalls/abort01: convert to " Sandeep Patil
@ 2019-03-26 11:58   ` Cyril Hrubis
  2019-03-26 12:47     ` Sandeep Patil
  0 siblings, 1 reply; 17+ messages in thread
From: Cyril Hrubis @ 2019-03-26 11:58 UTC (permalink / raw)
  To: ltp

Hi!
I've further simplified the test and pushed, thanks.

What I have done:

* Got rid of tst_brk(TFAIL, ...) calls
  see: https://github.com/linux-test-project/ltp/issues/462

* Used tst_strsig() and tst_strstatus() to print signal and status

* Used tst_res() API in the child

* Got rid of unused variables, etc.

diff --git a/testcases/kernel/syscalls/abort/abort01.c b/testcases/kernel/syscalls/abort/abort01.c
index ac5ddb140..386a22f26 100644
--- a/testcases/kernel/syscalls/abort/abort01.c
+++ b/testcases/kernel/syscalls/abort/abort01.c
@@ -27,51 +27,45 @@
 static void do_child(void)
 {
 	abort();
-	fprintf(stderr, "\tchild - abort failed.\n");
-	exit(1);
+	tst_res(TFAIL, "Abort returned");
+	exit(0);
 }
 
-void verify_abort(unsigned int nr)
+void verify_abort(void)
 {
-	int i;
-	int status, child, kidpid;
-	int sig, ex;
-	int core;
-	core = ex = sig = 0;
+	int status, kidpid;
+	int sig, core;
 
 	kidpid = SAFE_FORK();
 	if (kidpid == 0)
 		do_child();
 
-	child = SAFE_WAIT(&status);
-
-	if (WIFSIGNALED(status)) {
-		core = WCOREDUMP(status);
-		sig = WTERMSIG(status);
+	SAFE_WAIT(&status);
 
+	if (!WIFSIGNALED(status)) {
+		tst_res(TFAIL, "Child %s, expected SIGIOT",
+			tst_strstatus(status));
+		return;
 	}
 
-	if (WIFEXITED(status))
-		ex = WEXITSTATUS(status);
+	core = WCOREDUMP(status);
+	sig = WTERMSIG(status);
 
 	if (core == 0)
-		tst_brk(TFAIL,
-			"Missing core dump; exit(%d), signal(%d)",
-			ex, sig);
-	else if (core != -1)
+		tst_res(TFAIL, "abort() failed to dump core");
+	else
 		tst_res(TPASS, "abort() dumped core");
 
 	if (sig == SIGIOT)
 		tst_res(TPASS, "abort() raised SIGIOT");
 	else
-		tst_brk(TFAIL,
-			"Unexpected signal(%d), expected SIGIOT(%d)",
-			sig, SIGIOT);
+		tst_res(TFAIL, "abort() raised %s", tst_strsig(sig));
 }
 
+#define MIN_RLIMIT_CORE (1024 * 1024)
+
 static void setup(void)
 {
-#define MIN_RLIMIT_CORE (1024 * 1024)
 	struct rlimit rlim;
 
 	/* make sure we get core dumps */
@@ -83,9 +77,8 @@ static void setup(void)
 }
 
 static struct tst_test test = {
-	.tcnt = 3,
 	.needs_tmpdir = 1,
 	.forks_child = 1,
 	.setup = setup,
-	.test = verify_abort,
+	.test_all = verify_abort,
 };


-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [LTP] [PATCH 2/4] syscalls/accept01: convert to new library.
  2019-03-25 23:20 ` [LTP] [PATCH 2/4] syscalls/accept01: " Sandeep Patil
@ 2019-03-26 12:35   ` Cyril Hrubis
  2019-03-26 12:44     ` Sandeep Patil
  0 siblings, 1 reply; 17+ messages in thread
From: Cyril Hrubis @ 2019-03-26 12:35 UTC (permalink / raw)
  To: ltp

Hi!
> +static void setup_invalid_fd(struct test_case *tcase);
> +static void setup_nonsocket_fd(struct test_case *tcase);
> +static void setup_socket(struct test_case *tcase);
> +static void setup_fionbio(struct test_case *tcase);
> +static void cleanup_tcase(struct test_case *tcase);

Actually there is no need to have the setup & cleanup functions to be
executed at runtime, we can do all the test setup in the test setup and
cleanup in test cleanup.

We usually tend to solve this by making the fd in the testcase structure
a pointer and initializing it with a pointer to global variable that is
then intialized in setup. See for example syscalls/read/read02.c

> +/* test cases */

This is useless comment.

> +static struct test_case {
>  	int domain;		/* PF_INET, PF_UNIX, ... */
>  	int type;		/* SOCK_STREAM, SOCK_DGRAM ... */
>  	int proto;		/* protocol number (usually 0 = default) */
> +	int socketfd;		/* socketfd for the test case */
>  	struct sockaddr *sockaddr;	/* socket address buffer */
> -	socklen_t *salen;	/* accept's 3rd argument */
> +	socklen_t salen;	/* accept's 3rd argument */
>  	int retval;		/* syscall return value */
>  	int experrno;		/* expected errno */
> -	void (*setup) (void);
> -	void (*cleanup) (void);
> +	void (*setup) (struct test_case *);
> +	void (*cleanup) (struct test_case *);
>  	char *desc;
> -} tdat[] = {

...

> +		PF_INET, SOCK_STREAM, 0, -1, (struct sockaddr *)&fsin1,
> +		sizeof(fsin1), -1, EBADF, setup_invalid_fd, NULL,
> +		"bad file descriptor"
> +	},
> +	{
> +		PF_INET, SOCK_STREAM, 0, -1, (struct sockaddr *)&fsin1,
> +		sizeof(fsin1), -1, ENOTSOCK, setup_nonsocket_fd,
> +		cleanup_tcase, "bad file descriptor"
                                 ^
				 Better description would be:

                                 "fd is not socket"

> +	},
> +	{
> +		PF_INET, SOCK_STREAM, 0, -1, (struct sockaddr *)3,
> +		sizeof(fsin1), -1, EINVAL, setup_socket, cleanup_tcase,
> +		"invalid socket buffer"
> +	},
> +	{
> +		PF_INET, SOCK_STREAM, 0, -1, (struct sockaddr *)&fsin1,
> +		1, -1, EINVAL, setup_socket, cleanup_tcase,
> +		"invalid salen"
> +	},
> +	{
> +		PF_INET, SOCK_STREAM, 0, -1, (struct sockaddr *)&fsin1,
> +		sizeof(fsin1), -1, EINVAL, setup_fionbio, cleanup_tcase,
> +		"no queued connections"
> +	},
> +	{
> +		PF_INET, SOCK_DGRAM, 0, -1, (struct sockaddr *)&fsin1,
> +		sizeof(fsin1), -1, EOPNOTSUPP, setup_socket, cleanup_tcase,
> +		"UDP accept"
> +	},
> +};
>  
> -static void setup(void)
> -{
> -	TEST_PAUSE;
>  
> -	/* initialize local sockaddr */
> -	sin0.sin_family = AF_INET;
> -	sin0.sin_port = 0;
> -	sin0.sin_addr.s_addr = INADDR_ANY;
> -}
>  
> -static void cleanup(void)
> +static void setup_invalid_fd(struct test_case *tcase)
>  {
> +	tcase->socketfd = 400; /* anything that is not an open file */
>  }
>  
> -static void setup0(void)
> +static void setup_nonsocket_fd(struct test_case *tcase)
>  {
> -	if (tdat[testno].experrno == EBADF)
> -		s = 400;	/* anything not an open file */
> -	else if ((s = open("/dev/null", O_WRONLY)) == -1)
> -		tst_brkm(TBROK | TERRNO, cleanup, "error opening /dev/null");
> +	tcase->socketfd = SAFE_OPEN("/dev/null", O_WRONLY);
>  }
>  
> -static void cleanup0(void)
> +static void setup_socket(struct test_case *tcase)
>  {
> -	s = -1;
> +	tcase->socketfd = SAFE_SOCKET(tcase->domain, tcase->type, tcase->proto);
> +	SAFE_BIND(tcase->socketfd, (struct sockaddr *)&sin0, sizeof(sin0));
> +	tcase->salen = sizeof(fsin1);
>  }
>  
> -static void setup1(void)
> +static void setup_fionbio(struct test_case *tcase)
>  {
> -	s = SAFE_SOCKET(cleanup, tdat[testno].domain, tdat[testno].type,
> -			tdat[testno].proto);
> -	SAFE_BIND(cleanup, s, (struct sockaddr *)&sin0, sizeof(sin0));
> -	sinlen = sizeof(fsin1);
> +	int one = 1;
> +	setup_socket(tcase);
> +	SAFE_IOCTL(tcase->socketfd, FIONBIO, &one);
>  }
>  
> -static void cleanup1(void)
> +static void cleanup_tcase(struct test_case *tcase)
>  {
> -	(void)close(s);
> -	s = -1;
> +	if (tcase->socketfd >= 0)
> +		SAFE_CLOSE(tcase->socketfd);
> +
> +	tcase->socketfd = -1;
>  }
>  
> -static void setup2(void)
> +void verify_accept(unsigned int nr)
>  {
> -	setup1();		/* get a socket in s */
> -	sinlen = 1;		/* invalid s */
> +	struct test_case *tcase = &tcases[nr];
> +
> +	if (tcase->setup)
> +		tcase->setup(tcase);
> +
> +	TEST(accept(tcase->socketfd, tcase->sockaddr, &tcase->salen));
> +	if (TST_RET > 0) {
> +		/* don't leak accepted socket, close them first */
> +		SAFE_CLOSE(TST_RET);
> +		TST_RET = 0;
> +	}
> +
> +	if (TST_RET != tcase->retval ||
> +		(TST_RET < 0 && TST_ERR != tcase->experrno)) {
> +		tst_res(TFAIL, "%s ; returned"
> +				" %ld (expected %d), errno %d (expected"
> +				" %d)", tcase->desc, TST_RET, tcase->retval,
> +				TST_ERR, tcase->experrno);


It would be better to split this, since we are doing only error return
tests in this file we can do:


	if (TST_RET != -1) {
		tst_res(TFAIL, "%s: returned %i, expected -1", tcase->desc);
		return;
	}

	if (TST_ERR != tcase->experr) {
		tst_res(TFAIL | TTERRNO, "%s: expected %s", tcase->desc,
		        tst_strerrno(tcase->exp));
		return;
	}

	tst_res(TPASS | TTERRNO, "%s", tcase->desc);

> +	} else {
> +		tst_res(TPASS, "%s successful", tcase->desc);

Other than that it looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [LTP] [PATCH 2/4] syscalls/accept01: convert to new library.
  2019-03-26 12:35   ` Cyril Hrubis
@ 2019-03-26 12:44     ` Sandeep Patil
  0 siblings, 0 replies; 17+ messages in thread
From: Sandeep Patil @ 2019-03-26 12:44 UTC (permalink / raw)
  To: ltp

On Tue, Mar 26, 2019 at 01:35:19PM +0100, Cyril Hrubis wrote:
> Hi!
> > +static void setup_invalid_fd(struct test_case *tcase);
> > +static void setup_nonsocket_fd(struct test_case *tcase);
> > +static void setup_socket(struct test_case *tcase);
> > +static void setup_fionbio(struct test_case *tcase);
> > +static void cleanup_tcase(struct test_case *tcase);
> 
> Actually there is no need to have the setup & cleanup functions to be
> executed at runtime, we can do all the test setup in the test setup and
> cleanup in test cleanup.
> 
> We usually tend to solve this by making the fd in the testcase structure
> a pointer and initializing it with a pointer to global variable that is
> then intialized in setup. See for example syscalls/read/read02.c

Ack, I think this may have been a mix of both before, I can rework to use
globals easily instead.

> 
> > +/* test cases */
> 
> This is useless comment.

yes, will remove.
> 
> > +static struct test_case {
> >  	int domain;		/* PF_INET, PF_UNIX, ... */
> >  	int type;		/* SOCK_STREAM, SOCK_DGRAM ... */
> >  	int proto;		/* protocol number (usually 0 = default) */
> > +	int socketfd;		/* socketfd for the test case */
> >  	struct sockaddr *sockaddr;	/* socket address buffer */
> > -	socklen_t *salen;	/* accept's 3rd argument */
> > +	socklen_t salen;	/* accept's 3rd argument */
> >  	int retval;		/* syscall return value */
> >  	int experrno;		/* expected errno */
> > -	void (*setup) (void);
> > -	void (*cleanup) (void);
> > +	void (*setup) (struct test_case *);
> > +	void (*cleanup) (struct test_case *);
> >  	char *desc;
> > -} tdat[] = {
> 
> ...
> 
> > +		PF_INET, SOCK_STREAM, 0, -1, (struct sockaddr *)&fsin1,
> > +		sizeof(fsin1), -1, EBADF, setup_invalid_fd, NULL,
> > +		"bad file descriptor"
> > +	},
> > +	{
> > +		PF_INET, SOCK_STREAM, 0, -1, (struct sockaddr *)&fsin1,
> > +		sizeof(fsin1), -1, ENOTSOCK, setup_nonsocket_fd,
> > +		cleanup_tcase, "bad file descriptor"
>                                  ^
> 				 Better description would be:
> 
>                                  "fd is not socket"
> 
> > +	},
> > +	{
> > +		PF_INET, SOCK_STREAM, 0, -1, (struct sockaddr *)3,
> > +		sizeof(fsin1), -1, EINVAL, setup_socket, cleanup_tcase,
> > +		"invalid socket buffer"
> > +	},
> > +	{
> > +		PF_INET, SOCK_STREAM, 0, -1, (struct sockaddr *)&fsin1,
> > +		1, -1, EINVAL, setup_socket, cleanup_tcase,
> > +		"invalid salen"
> > +	},
> > +	{
> > +		PF_INET, SOCK_STREAM, 0, -1, (struct sockaddr *)&fsin1,
> > +		sizeof(fsin1), -1, EINVAL, setup_fionbio, cleanup_tcase,
> > +		"no queued connections"
> > +	},
> > +	{
> > +		PF_INET, SOCK_DGRAM, 0, -1, (struct sockaddr *)&fsin1,
> > +		sizeof(fsin1), -1, EOPNOTSUPP, setup_socket, cleanup_tcase,
> > +		"UDP accept"
> > +	},
> > +};
> >  
> > -static void setup(void)
> > -{
> > -	TEST_PAUSE;
> >  
> > -	/* initialize local sockaddr */
> > -	sin0.sin_family = AF_INET;
> > -	sin0.sin_port = 0;
> > -	sin0.sin_addr.s_addr = INADDR_ANY;
> > -}
> >  
> > -static void cleanup(void)
> > +static void setup_invalid_fd(struct test_case *tcase)
> >  {
> > +	tcase->socketfd = 400; /* anything that is not an open file */
> >  }
> >  
> > -static void setup0(void)
> > +static void setup_nonsocket_fd(struct test_case *tcase)
> >  {
> > -	if (tdat[testno].experrno == EBADF)
> > -		s = 400;	/* anything not an open file */
> > -	else if ((s = open("/dev/null", O_WRONLY)) == -1)
> > -		tst_brkm(TBROK | TERRNO, cleanup, "error opening /dev/null");
> > +	tcase->socketfd = SAFE_OPEN("/dev/null", O_WRONLY);
> >  }
> >  
> > -static void cleanup0(void)
> > +static void setup_socket(struct test_case *tcase)
> >  {
> > -	s = -1;
> > +	tcase->socketfd = SAFE_SOCKET(tcase->domain, tcase->type, tcase->proto);
> > +	SAFE_BIND(tcase->socketfd, (struct sockaddr *)&sin0, sizeof(sin0));
> > +	tcase->salen = sizeof(fsin1);
> >  }
> >  
> > -static void setup1(void)
> > +static void setup_fionbio(struct test_case *tcase)
> >  {
> > -	s = SAFE_SOCKET(cleanup, tdat[testno].domain, tdat[testno].type,
> > -			tdat[testno].proto);
> > -	SAFE_BIND(cleanup, s, (struct sockaddr *)&sin0, sizeof(sin0));
> > -	sinlen = sizeof(fsin1);
> > +	int one = 1;
> > +	setup_socket(tcase);
> > +	SAFE_IOCTL(tcase->socketfd, FIONBIO, &one);
> >  }
> >  
> > -static void cleanup1(void)
> > +static void cleanup_tcase(struct test_case *tcase)
> >  {
> > -	(void)close(s);
> > -	s = -1;
> > +	if (tcase->socketfd >= 0)
> > +		SAFE_CLOSE(tcase->socketfd);
> > +
> > +	tcase->socketfd = -1;
> >  }
> >  
> > -static void setup2(void)
> > +void verify_accept(unsigned int nr)
> >  {
> > -	setup1();		/* get a socket in s */
> > -	sinlen = 1;		/* invalid s */
> > +	struct test_case *tcase = &tcases[nr];
> > +
> > +	if (tcase->setup)
> > +		tcase->setup(tcase);
> > +
> > +	TEST(accept(tcase->socketfd, tcase->sockaddr, &tcase->salen));
> > +	if (TST_RET > 0) {
> > +		/* don't leak accepted socket, close them first */
> > +		SAFE_CLOSE(TST_RET);
> > +		TST_RET = 0;
> > +	}
> > +
> > +	if (TST_RET != tcase->retval ||
> > +		(TST_RET < 0 && TST_ERR != tcase->experrno)) {
> > +		tst_res(TFAIL, "%s ; returned"
> > +				" %ld (expected %d), errno %d (expected"
> > +				" %d)", tcase->desc, TST_RET, tcase->retval,
> > +				TST_ERR, tcase->experrno);
> 
> 
> It would be better to split this, since we are doing only error return
> tests in this file we can do:
> 
> 
> 	if (TST_RET != -1) {
> 		tst_res(TFAIL, "%s: returned %i, expected -1", tcase->desc);
> 		return;
> 	}
> 
> 	if (TST_ERR != tcase->experr) {
> 		tst_res(TFAIL | TTERRNO, "%s: expected %s", tcase->desc,
> 		        tst_strerrno(tcase->exp));
> 		return;
> 	}
> 
> 	tst_res(TPASS | TTERRNO, "%s", tcase->desc);
> 
> > +	} else {
> > +		tst_res(TPASS, "%s successful", tcase->desc);
> 
> Other than that it looks good.

Thanks, I'll rework and send this.

- ssp

> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [LTP] [RFC PATCH 1/4] syscalls/abort01: convert to new library
  2019-03-26 11:58   ` Cyril Hrubis
@ 2019-03-26 12:47     ` Sandeep Patil
  2019-03-26 12:51       ` Cyril Hrubis
  0 siblings, 1 reply; 17+ messages in thread
From: Sandeep Patil @ 2019-03-26 12:47 UTC (permalink / raw)
  To: ltp

On Tue, Mar 26, 2019 at 12:58:25PM +0100, Cyril Hrubis wrote:
> Hi!
> I've further simplified the test and pushed, thanks.
> 
> What I have done:
> 
> * Got rid of tst_brk(TFAIL, ...) calls
>   see: https://github.com/linux-test-project/ltp/issues/462

Thanks for this, it is good to know. What is the recommended replacement?
tst_res()?

> 
> * Used tst_strsig() and tst_strstatus() to print signal and status
> 
> * Used tst_res() API in the child
> 
> * Got rid of unused variables, etc.

I am surprised that didn't throw a warning + build error for me.
other than that, thanks for doing this

> 
> diff --git a/testcases/kernel/syscalls/abort/abort01.c b/testcases/kernel/syscalls/abort/abort01.c
> index ac5ddb140..386a22f26 100644
> --- a/testcases/kernel/syscalls/abort/abort01.c
> +++ b/testcases/kernel/syscalls/abort/abort01.c
> @@ -27,51 +27,45 @@
>  static void do_child(void)
>  {
>  	abort();
> -	fprintf(stderr, "\tchild - abort failed.\n");
> -	exit(1);
> +	tst_res(TFAIL, "Abort returned");
> +	exit(0);
>  }
>  
> -void verify_abort(unsigned int nr)
> +void verify_abort(void)
>  {
> -	int i;
> -	int status, child, kidpid;
> -	int sig, ex;
> -	int core;
> -	core = ex = sig = 0;
> +	int status, kidpid;
> +	int sig, core;
>  
>  	kidpid = SAFE_FORK();
>  	if (kidpid == 0)
>  		do_child();
>  
> -	child = SAFE_WAIT(&status);
> -
> -	if (WIFSIGNALED(status)) {
> -		core = WCOREDUMP(status);
> -		sig = WTERMSIG(status);
> +	SAFE_WAIT(&status);
>  
> +	if (!WIFSIGNALED(status)) {
> +		tst_res(TFAIL, "Child %s, expected SIGIOT",
> +			tst_strstatus(status));
> +		return;
>  	}
>  
> -	if (WIFEXITED(status))
> -		ex = WEXITSTATUS(status);
> +	core = WCOREDUMP(status);
> +	sig = WTERMSIG(status);
>  
>  	if (core == 0)
> -		tst_brk(TFAIL,
> -			"Missing core dump; exit(%d), signal(%d)",
> -			ex, sig);
> -	else if (core != -1)
> +		tst_res(TFAIL, "abort() failed to dump core");
> +	else
>  		tst_res(TPASS, "abort() dumped core");
>  
>  	if (sig == SIGIOT)
>  		tst_res(TPASS, "abort() raised SIGIOT");
>  	else
> -		tst_brk(TFAIL,
> -			"Unexpected signal(%d), expected SIGIOT(%d)",
> -			sig, SIGIOT);
> +		tst_res(TFAIL, "abort() raised %s", tst_strsig(sig));
>  }
>  
> +#define MIN_RLIMIT_CORE (1024 * 1024)
> +
>  static void setup(void)
>  {
> -#define MIN_RLIMIT_CORE (1024 * 1024)
>  	struct rlimit rlim;
>  
>  	/* make sure we get core dumps */
> @@ -83,9 +77,8 @@ static void setup(void)
>  }
>  
>  static struct tst_test test = {
> -	.tcnt = 3,
>  	.needs_tmpdir = 1,
>  	.forks_child = 1,
>  	.setup = setup,
> -	.test = verify_abort,
> +	.test_all = verify_abort,
>  };
> 
> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [LTP] [PATCH 0/4] Convert tests to use new library
  2019-03-26  9:42 ` [LTP] [PATCH 0/4] Convert tests to use " Cyril Hrubis
@ 2019-03-26 12:48   ` Sandeep Patil
  0 siblings, 0 replies; 17+ messages in thread
From: Sandeep Patil @ 2019-03-26 12:48 UTC (permalink / raw)
  To: ltp

On Tue, Mar 26, 2019 at 10:42:28AM +0100, Cyril Hrubis wrote:
> Hi!
> > I've only tested these with glibc + x86 vm. I removed some of the
> > UCLINUX specific ifdefs in the process that I am not so sure about.
> > Is there a way for me to test these on other systems?
> 
> The uClinux support has been deprecated and we are slowly removing the
> support. The uClinux project itself seems to be dead so it does not make
> sense to keep supporting it.

Great, good to know. I am literally going through tests in alphabetical
orders and convert as many as I can in this sprint. This information helps to
clean things up in the process too ..

- ssp

> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [LTP] [RFC PATCH 1/4] syscalls/abort01: convert to new library
  2019-03-26 12:47     ` Sandeep Patil
@ 2019-03-26 12:51       ` Cyril Hrubis
  2019-03-28  4:07         ` Sandeep Patil
  0 siblings, 1 reply; 17+ messages in thread
From: Cyril Hrubis @ 2019-03-26 12:51 UTC (permalink / raw)
  To: ltp

Hi!
> > * Got rid of tst_brk(TFAIL, ...) calls
> >   see: https://github.com/linux-test-project/ltp/issues/462
> 
> Thanks for this, it is good to know. What is the recommended replacement?
> tst_res()?

Yes, tst_res() followed by either return or exit(0) if you need to
actually exit the code flow.

> > 
> > * Used tst_strsig() and tst_strstatus() to print signal and status
> > 
> > * Used tst_res() API in the child
> > 
> > * Got rid of unused variables, etc.
> 
> I am surprised that didn't throw a warning + build error for me.
> other than that, thanks for doing this

That depends on gcc version and mix of warning flags...

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [LTP] [PATCH 3/4] syscalls/accept4: convert to new library
  2019-03-25 23:20 ` [LTP] [PATCH 3/4] syscalls/accept4: " Sandeep Patil
@ 2019-03-26 15:33   ` Cyril Hrubis
  0 siblings, 0 replies; 17+ messages in thread
From: Cyril Hrubis @ 2019-03-26 15:33 UTC (permalink / raw)
  To: ltp

Hi!
Pushed with small cleanup and simplification, thanks.

diff --git a/testcases/kernel/syscalls/accept4/accept4_01.c b/testcases/kernel/syscalls/accept4/accept4_01.c
index 036457571..c423b2a7d 100644
--- a/testcases/kernel/syscalls/accept4/accept4_01.c
+++ b/testcases/kernel/syscalls/accept4/accept4_01.c
@@ -25,18 +25,18 @@
 #define PORT_NUM 33333
 
 #ifndef SOCK_CLOEXEC
-#define SOCK_CLOEXEC    O_CLOEXEC
+# define SOCK_CLOEXEC    O_CLOEXEC
 #endif
 #ifndef SOCK_NONBLOCK
-#define SOCK_NONBLOCK   O_NONBLOCK
+# define SOCK_NONBLOCK   O_NONBLOCK
 #endif
 
 #if defined(SYS_ACCEPT4)	/* the socketcall() number */
 #define USE_SOCKETCALL 1
 #endif
 
-struct sockaddr_in conn_addr;
-int listening_fd;
+static struct sockaddr_in conn_addr;
+static int listening_fd;
 
 #if !(__GLIBC_PREREQ(2, 10))
 static int
@@ -78,7 +78,7 @@ static int create_listening_socket(void)
 	return lfd;
 }
 
-void setup(void)
+static void setup(void)
 {
 	memset(&conn_addr, 0, sizeof(struct sockaddr_in));
 	conn_addr.sin_family = AF_INET;
@@ -88,7 +88,7 @@ void setup(void)
 	listening_fd = create_listening_socket();
 }
 
-void cleanup(void)
+static void cleanup(void)
 {
 	SAFE_CLOSE(listening_fd);
 }
@@ -103,11 +103,11 @@ static struct test_case {
 	{ SOCK_CLOEXEC, SOCK_NONBLOCK },
 };
 
-void verify_accept4(unsigned int nr)
+static void verify_accept4(unsigned int nr)
 {
 	struct test_case *tcase = &tcases[nr];
 	int connfd, acceptfd;
-	int fdf, flf, fdf_pass, flf_pass;
+	int fdf, flf, fdf_pass, flf_pass, fd_cloexec, fd_nonblock;
 	struct sockaddr_in claddr;
 	socklen_t addrlen;
 
@@ -133,27 +133,29 @@ void verify_accept4(unsigned int nr)
 
 	/* Test to see if O_CLOEXEC is as expected */
 	fdf = SAFE_FCNTL(acceptfd, F_GETFD);
-	fdf_pass = !!(fdf & FD_CLOEXEC) == !!(tcase->cloexec & SOCK_CLOEXEC);
-	if (!fdf_pass)
-		tst_res(TFAIL,  "Close-on-exec flag mismatch, %d vs %d",
-			 !!(fdf & FD_CLOEXEC),
-			 !!(tcase->cloexec & SOCK_CLOEXEC));
+	fd_cloexec = !!(fdf & FD_CLOEXEC);
+	fdf_pass = fd_cloexec == !!tcase->cloexec;
+	if (!fdf_pass) {
+		tst_res(TFAIL, "Close-on-exec flag mismatch, %d vs %d",
+			fd_cloexec, !!tcase->cloexec);
+	}
 
 	/* Test to see if O_NONBLOCK is as expected */
 	flf = SAFE_FCNTL(acceptfd, F_GETFL);
-	flf_pass = !!(flf & O_NONBLOCK) == !!(tcase->nonblock & SOCK_NONBLOCK);
-	if (!flf_pass)
+	fd_nonblock = !!(flf & O_NONBLOCK);
+	flf_pass = fd_nonblock == !!tcase->nonblock;
+	if (!flf_pass) {
 		tst_res(TFAIL, "nonblock flag mismatch, %d vs %d",
-			 !!(fdf & O_NONBLOCK),
-			 !!(tcase->nonblock & SOCK_NONBLOCK));
+		        fd_nonblock, !!tcase->nonblock);
+	}
 
 	SAFE_CLOSE(acceptfd);
 	SAFE_CLOSE(connfd);
 
-	if (fdf_pass && flf_pass)
-		tst_res(TPASS, "Test passed: Close-on-exec %d, nonblock %d",
-				fdf_pass, flf_pass);
-
+	if (fdf_pass && flf_pass) {
+		tst_res(TPASS, "Close-on-exec %d, nonblock %d",
+				fd_cloexec, fd_nonblock);
+	}
 }
 
 static struct tst_test test = {

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [LTP] [PATCH 4/4] syscalls/acct01: convert to new library
  2019-03-25 23:20 ` [LTP] [PATCH 4/4] syscalls/acct01: " Sandeep Patil
@ 2019-03-26 16:11   ` Cyril Hrubis
  0 siblings, 0 replies; 17+ messages in thread
From: Cyril Hrubis @ 2019-03-26 16:11 UTC (permalink / raw)
  To: ltp

Hi!
>  #define DIR_MODE	(S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP| \
>  			 S_IXGRP|S_IROTH|S_IXOTH)
> @@ -48,169 +28,106 @@
>  #define TEST_FILE5	"./tmpfile"
>  #define TEST_FILE6	"test_file_eloop1"
>  #define TEST_FILE7	nametoolong
> -#define TEST_FILE8	"mntpoint/tmp"
> +#define TEST_FILE8	"mntpoint/file"

Maybe it would be better to name these after the error they cause i.e.
FILE_EISDIR etc. Also some of these do point outside of the test
temporary directory and even if it's unlikely to mess up with the system
that way we should use "." instead of "/", create a file in PWD instead
of /etc/fstab/ etc.

...

> -	tst_rmdir();
> +	if (TST_ERR == tcase->exp_errno)
> +		tst_res(TPASS | TTERRNO, "acct() failed as expected");
> +	else
> +		tst_res(TFAIL | TTERRNO,
> +				"acct() failed, expected: %d - %s",
> +				tcase->exp_errno, strerror(tcase->exp_errno));

As in previous test you should use tst_strerrno() to print errno
instead.


Other than these two minor points the rest looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [LTP] [RFC PATCH 1/4] syscalls/abort01: convert to new library
  2019-03-26 12:51       ` Cyril Hrubis
@ 2019-03-28  4:07         ` Sandeep Patil
  2019-04-03 12:06           ` Cyril Hrubis
  0 siblings, 1 reply; 17+ messages in thread
From: Sandeep Patil @ 2019-03-28  4:07 UTC (permalink / raw)
  To: ltp

On Tue, Mar 26, 2019 at 01:51:32PM +0100, Cyril Hrubis wrote:
> Hi!
> > > * Got rid of tst_brk(TFAIL, ...) calls
> > >   see: https://github.com/linux-test-project/ltp/issues/462
> > 
> > Thanks for this, it is good to know. What is the recommended replacement?
> > tst_res()?
> 
> Yes, tst_res() followed by either return or exit(0) if you need to
> actually exit the code flow.
> 
> > > 
> > > * Used tst_strsig() and tst_strstatus() to print signal and status
> > > 
> > > * Used tst_res() API in the child
> > > 
> > > * Got rid of unused variables, etc.
> > 
> > I am surprised that didn't throw a warning + build error for me.
> > other than that, thanks for doing this
> 
> That depends on gcc version and mix of warning flags...

clang actually throws a lot of warnings for us when we build it natively for
Android. In this case however, I just used my laptop "gcc (Debian 7.3.0-5)"
and default flags for LTP builds.

Do you have plans of enabling -Werror?

- ssp

> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [LTP] [RFC PATCH 1/4] syscalls/abort01: convert to new library
  2019-03-28  4:07         ` Sandeep Patil
@ 2019-04-03 12:06           ` Cyril Hrubis
  2019-04-03 15:49             ` Sandeep Patil
  0 siblings, 1 reply; 17+ messages in thread
From: Cyril Hrubis @ 2019-04-03 12:06 UTC (permalink / raw)
  To: ltp

Hi!
> > > I am surprised that didn't throw a warning + build error for me.
> > > other than that, thanks for doing this
> > 
> > That depends on gcc version and mix of warning flags...
> 
> clang actually throws a lot of warnings for us when we build it natively for
> Android. In this case however, I just used my laptop "gcc (Debian 7.3.0-5)"
> and default flags for LTP builds.
> 
> Do you have plans of enabling -Werror?

We cannot do that with the amount of legacy code we have the build would
fail pretty much every time. So no, no plans for the foreseeable future.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [LTP] [RFC PATCH 1/4] syscalls/abort01: convert to new library
  2019-04-03 12:06           ` Cyril Hrubis
@ 2019-04-03 15:49             ` Sandeep Patil
  0 siblings, 0 replies; 17+ messages in thread
From: Sandeep Patil @ 2019-04-03 15:49 UTC (permalink / raw)
  To: ltp

On Wed, Apr 03, 2019 at 02:06:53PM +0200, Cyril Hrubis wrote:
> Hi!
> > > > I am surprised that didn't throw a warning + build error for me.
> > > > other than that, thanks for doing this
> > > 
> > > That depends on gcc version and mix of warning flags...
> > 
> > clang actually throws a lot of warnings for us when we build it natively for
> > Android. In this case however, I just used my laptop "gcc (Debian 7.3.0-5)"
> > and default flags for LTP builds.
> > 
> > Do you have plans of enabling -Werror?
> 
> We cannot do that with the amount of legacy code we have the build would
> fail pretty much every time. So no, no plans for the foreseeable future.

Ack, thanks Cyril. I'd rather convert as many tests to the new library before
I get to them then. By the way, I sent V2s for acct01 & accept01 addressing
your comments now.

- ssp

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2019-04-03 15:49 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-25 23:20 [LTP] [PATCH 0/4] Convert tests to use new library Sandeep Patil
2019-03-25 23:20 ` [LTP] [RFC PATCH 1/4] syscalls/abort01: convert to " Sandeep Patil
2019-03-26 11:58   ` Cyril Hrubis
2019-03-26 12:47     ` Sandeep Patil
2019-03-26 12:51       ` Cyril Hrubis
2019-03-28  4:07         ` Sandeep Patil
2019-04-03 12:06           ` Cyril Hrubis
2019-04-03 15:49             ` Sandeep Patil
2019-03-25 23:20 ` [LTP] [PATCH 2/4] syscalls/accept01: " Sandeep Patil
2019-03-26 12:35   ` Cyril Hrubis
2019-03-26 12:44     ` Sandeep Patil
2019-03-25 23:20 ` [LTP] [PATCH 3/4] syscalls/accept4: " Sandeep Patil
2019-03-26 15:33   ` Cyril Hrubis
2019-03-25 23:20 ` [LTP] [PATCH 4/4] syscalls/acct01: " Sandeep Patil
2019-03-26 16:11   ` Cyril Hrubis
2019-03-26  9:42 ` [LTP] [PATCH 0/4] Convert tests to use " Cyril Hrubis
2019-03-26 12:48   ` Sandeep Patil

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.