Linux Test Project
 help / color / mirror / Atom feed
* [LTP] [PATCH 0/3 v2] syscalls/close: Convert close{01, 02, 08} to the new API
@ 2021-04-26 12:52 Xie Ziyao
  2021-04-26 12:52 ` [LTP] [PATCH 1/3 v2] syscalls/close: Convert close01 " Xie Ziyao
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Xie Ziyao @ 2021-04-26 12:52 UTC (permalink / raw)
  To: ltp

Xie Ziyao (3):
  syscalls/close: Convert close01 to the new API
  syscalls/close: Convert close02 to the new API
  syscalls/close: Convert close08 to the new API

 testcases/kernel/syscalls/close/close01.c | 134 +++++-----------
 testcases/kernel/syscalls/close/close02.c | 122 ++------------
 testcases/kernel/syscalls/close/close08.c | 186 +++-------------------
 3 files changed, 76 insertions(+), 366 deletions(-)

--
2.17.1


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

* [LTP] [PATCH 1/3 v2] syscalls/close: Convert close01 to the new API
  2021-04-26 12:52 [LTP] [PATCH 0/3 v2] syscalls/close: Convert close{01, 02, 08} to the new API Xie Ziyao
@ 2021-04-26 12:52 ` Xie Ziyao
  2021-04-27 12:15   ` Cyril Hrubis
  2021-04-26 12:52 ` [LTP] [PATCH 2/3 v2] syscalls/close: Convert close02 " Xie Ziyao
  2021-04-26 12:52 ` [LTP] [PATCH 3/3 v2] syscalls/close: Convert close08 " Xie Ziyao
  2 siblings, 1 reply; 8+ messages in thread
From: Xie Ziyao @ 2021-04-26 12:52 UTC (permalink / raw)
  To: ltp

1. Cleanup and convert close01 to the new API;
2. Add SAFE_CLOSE() to the file descriptor.

Signed-off-by: Xie Ziyao <xieziyao@huawei.com>
---
v1->v2:
1. Cleanup with TST_EXP_PASS();
2. Add SAFE_CLOSE() to the file descriptor for a pipe.

 testcases/kernel/syscalls/close/close01.c | 134 +++++++---------------
 1 file changed, 40 insertions(+), 94 deletions(-)

diff --git a/testcases/kernel/syscalls/close/close01.c b/testcases/kernel/syscalls/close/close01.c
index c734ff7d2..759c43af0 100644
--- a/testcases/kernel/syscalls/close/close01.c
+++ b/testcases/kernel/syscalls/close/close01.c
@@ -1,124 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) International Business Machines  Corp., 2001
- *  07/2001 Ported by Wayne Boyer
+ * 07/2001 Ported by Wayne Boyer
+ */
+
+/*\
+ * [Description]
  *
- * 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.
+ * Basic test for the close() syscall.
  *
- * 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.
+ * [Description]
  *
- * 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
- *	Test that closing a regular file and a pipe works correctly
+ * Test that closing a regular file and a pipe works correctly.
  */

 #include <stdio.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <sys/stat.h>
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"

-void cleanup(void);
-void setup(void);
+#define FILENAME "close01_testfile"

-char *TCID = "close01";
-int TST_TOTAL = 2;
+int fild, newfd, pipefildes[2];

-char fname[40] = "";
+static void setup_file(void)
+{
+	newfd = SAFE_DUP(fild);
+}

-int fild, newfd, pipefildes[2];
+static void setup_pipe(void)
+{
+	SAFE_PIPE(pipefildes);
+	SAFE_CLOSE(pipefildes[1]);
+}

 struct test_case_t {
 	int *fd;
 	char *type;
-} TC[] = {
-	/* file descriptor for a regular file */
-	{
-	&newfd, "file"},
-	    /* file descriptor for a pipe */
-	{
-	&pipefildes[0], "pipe"}
+	void (*setupfunc) ();
+} tc[] = {
+	{&newfd, "file", setup_file},
+	{&pipefildes[0], "pipe", setup_pipe}
 };

-int main(int ac, char **av)
+static void run(unsigned int i)
 {
-
-	int i;
-	int lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		tst_count = 0;
-
-		if ((fild = creat(fname, 0777)) == -1)
-			tst_brkm(TBROK | TERRNO, cleanup, "can't open file %s",
-				 fname);
-
-		if ((newfd = dup(fild)) == -1)
-			tst_brkm(TBROK | TERRNO, cleanup,
-				 "can't dup the file des");
-
-		SAFE_PIPE(cleanup, pipefildes);
-
-		for (i = 0; i < TST_TOTAL; i++) {
-
-			TEST(close(*TC[i].fd));
-
-			if (TEST_RETURN == -1) {
-				tst_resm(TFAIL, "call failed unexpectedly");
-				continue;
-			}
-
-			if (close(*TC[i].fd) == -1) {
-				tst_resm(TPASS, "%s appears closed",
-					 TC[i].type);
-			} else {
-				tst_resm(TFAIL, "%s close succeeded on"
-					 "second attempt", TC[i].type);
-			}
-		}
-
-	}
-
-	cleanup();
-	tst_exit();
+	tc[i].setupfunc();
+	TST_EXP_PASS(close(*tc[i].fd), "close a %s", tc[i].type);
 }

-void setup(void)
+static void setup(void)
 {
-	int mypid;
-
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-
 	umask(0);
-
-	TEST_PAUSE;
-
-	tst_tmpdir();
-
-	mypid = getpid();
-	sprintf(fname, "fname.%d", mypid);
+	fild = SAFE_CREAT(FILENAME, 0777);
 }

-void cleanup(void)
+static void cleanup(void)
 {
-	close(fild);
-
-	tst_rmdir();
-
+	SAFE_CLOSE(fild);
 }
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tc),
+	.needs_tmpdir = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test = run,
+};
--
2.17.1


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

* [LTP] [PATCH 2/3 v2] syscalls/close: Convert close02 to the new API
  2021-04-26 12:52 [LTP] [PATCH 0/3 v2] syscalls/close: Convert close{01, 02, 08} to the new API Xie Ziyao
  2021-04-26 12:52 ` [LTP] [PATCH 1/3 v2] syscalls/close: Convert close01 " Xie Ziyao
@ 2021-04-26 12:52 ` Xie Ziyao
  2021-04-27 12:20   ` Cyril Hrubis
  2021-04-26 12:52 ` [LTP] [PATCH 3/3 v2] syscalls/close: Convert close08 " Xie Ziyao
  2 siblings, 1 reply; 8+ messages in thread
From: Xie Ziyao @ 2021-04-26 12:52 UTC (permalink / raw)
  To: ltp

Cleanup and convert close02 to the new API.

Signed-off-by: Xie Ziyao <xieziyao@huawei.com>
---
v1->v2:
Cleanup with TST_EXP_FAIL() and remove setup() function.

 testcases/kernel/syscalls/close/close02.c | 122 +++-------------------
 1 file changed, 15 insertions(+), 107 deletions(-)

diff --git a/testcases/kernel/syscalls/close/close02.c b/testcases/kernel/syscalls/close/close02.c
index ddda5d9ec..50ca8006f 100644
--- a/testcases/kernel/syscalls/close/close02.c
+++ b/testcases/kernel/syscalls/close/close02.c
@@ -1,121 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *
- *   Copyright (c) International Business Machines  Corp., 2001
- *
- *   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
+ * Copyright (c) International Business Machines  Corp., 2001
+ * 07/2001 Ported by Wayne Boyer
  */

-/*
- * NAME
- * 	close02.c
- *
- * DESCRIPTION
- * 	Check that an invalid file descriptor returns EBADF
- *
- * ALGORITHM
- *	loop if that option is specified
- * 	   call close using the TEST macro and passing in an invalid fd
- *	   if the call succeedes
- *	      issue a FAIL message
- *	   else
- *	      log the errno
- *	      if the errno == EBADF
- *	         issue a PASS message
- *	      else
- *	         issue a FAIL message
- *	cleanup
- *
- * USAGE:  <for command-line>
- *  close02 [-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.
+/*\
+ * [Description]
+ *
+ * Basic test for the close() syscall.
  *
- * HISTORY
- *	07/2001 Ported by Wayne Boyer
+ * [Algorithm]
  *
- * RESTRICTIONS
- * 	None
+ * Call close(-1) and expects it to return EBADF.
  */

 #include <stdio.h>
 #include <errno.h>
 #include <sys/stat.h>
-#include "test.h"
+#include "tst_test.h"

-void cleanup(void);
-void setup(void);
-
-char *TCID = "close02";
-int TST_TOTAL = 1;
-
-int main(int ac, char **av)
+static void run(void)
 {
-	int lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	/* The following loop checks looping state if -i option given */
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		/* reset tst_count in case we are looping */
-		tst_count = 0;
-
-		TEST(close(-1));
-
-		if (TEST_RETURN != -1) {
-			tst_resm(TFAIL, "Closed a non existent fildes");
-		} else {
-			if (TEST_ERRNO != EBADF) {
-				tst_resm(TFAIL, "close() FAILED to set errno "
-					 "to EBADF on an invalid fd, got %d",
-					 errno);
-			} else {
-				tst_resm(TPASS, "call returned EBADF");
-			}
-		}
-	}
-	cleanup();
-
-	tst_exit();
-
+	TST_EXP_FAIL(close(-1), EBADF);
 }

-/*
- * setup() - performs all ONE TIME setup for this test
- */
-void setup(void)
-{
-
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-
-	umask(0);
-
-	TEST_PAUSE;
-}
-
-/*
- * cleanup() - performs all the ONE TIME cleanup for this test at completion
- * or premature exit.
- */
-void cleanup(void)
-{
-
-}
+static struct tst_test test = {
+	.test_all = run,
+};
--
2.17.1


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

* [LTP] [PATCH 3/3 v2] syscalls/close: Convert close08 to the new API
  2021-04-26 12:52 [LTP] [PATCH 0/3 v2] syscalls/close: Convert close{01, 02, 08} to the new API Xie Ziyao
  2021-04-26 12:52 ` [LTP] [PATCH 1/3 v2] syscalls/close: Convert close01 " Xie Ziyao
  2021-04-26 12:52 ` [LTP] [PATCH 2/3 v2] syscalls/close: Convert close02 " Xie Ziyao
@ 2021-04-26 12:52 ` Xie Ziyao
  2021-04-27 12:23   ` Cyril Hrubis
  2 siblings, 1 reply; 8+ messages in thread
From: Xie Ziyao @ 2021-04-26 12:52 UTC (permalink / raw)
  To: ltp

Cleanup and syscalls/close: Convert close08 to the new API.

Signed-off-by: Xie Ziyao <xieziyao@huawei.com>
---
v1->v2:
Cleanup with TST_EXP_PASS() and remove setup() function.

 testcases/kernel/syscalls/close/close08.c | 186 +++-------------------
 1 file changed, 21 insertions(+), 165 deletions(-)

diff --git a/testcases/kernel/syscalls/close/close08.c b/testcases/kernel/syscalls/close/close08.c
index ccdefa173..c4ee52060 100644
--- a/testcases/kernel/syscalls/close/close08.c
+++ b/testcases/kernel/syscalls/close/close08.c
@@ -1,177 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * 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.
- *
- * Further, this software is distributed without any warranty that it is
- * free of the rightful claim of any third person regarding infringement
- * or the like.  Any license provided herein, whether implied or
- * otherwise, applies only to this software file.  Patent licenses, if
- * any, provided herein do not apply to combinations of this program with
- * other software, or any other product whatsoever.
- *
- * 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 Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Contact information: Silicon Graphics, Inc., 1600 Amphitheatre Pkwy,
- * Mountain View, CA  94043, or:
- *
- * http://www.sgi.com
- *
- * For further information regarding this notice, see:
- *
- * http://oss.sgi.com/projects/GenInfo/NoticeExplan/
- *
+ * AUTHOR: William Roske
+ * CO-PILOT: Dave Fenner
  */
-/* $Id: close08.c,v 1.6 2009/10/13 14:00:46 subrata_modak Exp $ */
-/**********************************************************
- *
- *    OS Test - Silicon Graphics, Inc.
- *
- *    TEST IDENTIFIER	: close08
- *
- *    EXECUTED BY	: anyone
- *
- *    TEST TITLE	: Basic test for close(2)
- *
- *    PARENT DOCUMENT	: usctpl01
- *
- *    TEST CASE TOTAL	: 1
- *
- *    WALL CLOCK TIME	: 1
- *
- *    CPU TYPES		: ALL
- *
- *    AUTHOR		: William Roske
- *
- *    CO-PILOT		: Dave Fenner
- *
- *    DATE STARTED	: 03/30/92
- *
- *    INITIAL RELEASE	: UNICOS 7.0
- *
- *    TEST CASES
- *
- * 	1.) close(2) returns...(See Description)
- *
- *    INPUT SPECIFICATIONS
- * 	The standard options for system call tests are accepted.
- *	(See the parse_opts(3) man page).
- *
- *    OUTPUT SPECIFICATIONS
- *$
- *    DURATION
- * 	Terminates - with frequency and infinite modes.
- *
- *    SIGNALS
- * 	Uses SIGUSR1 to pause before test if option set.
- * 	(See the parse_opts(3) man page).
- *
- *    RESOURCES
- * 	None
- *
- *    ENVIRONMENTAL NEEDS
- *      No run-time environmental needs.
- *
- *    SPECIAL PROCEDURAL REQUIREMENTS
- * 	None
- *
- *    INTERCASE DEPENDENCIES
- * 	None
- *
- *    DETAILED DESCRIPTION
- *	This is a Phase I test for the close(2) system call.  It is intended
- *	to provide a limited exposure of the system call, for now.  It
- *	should/will be extended when full functional tests are written for
- *	close(2).
- *
- * 	Setup:
- * 	  Setup signal handling.
- *	  Pause for SIGUSR1 if option specified.
- *
- * 	Test:
- *	 Loop if the proper options are given.
- * 	  Execute system call
- *	  Check return code, if system call failed (return=-1)
- *		Log the errno and Issue a FAIL message.
- *	  Otherwise, Issue a PASS message.
- *
- * 	Cleanup:
- * 	  Print errno log and/or timing stats if options given
+
+/*\
+ * [Description]
+ * Basic test for the close() syscall.
  *
+ * [Algorithm]
  *
- *#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#**/
+ * Call close() and expects it to succeed.
+ */

-#include <sys/types.h>
+#include <stdio.h>
 #include <fcntl.h>
-#include <errno.h>
-#include <string.h>
-#include <signal.h>
-#include "test.h"
-#include "safe_macros.h"
-
-void setup();
-void cleanup();
-
-char *TCID = "close08";
-int TST_TOTAL = 1;
-
-char fname[255];
-int fd;
-
-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;
-
-		if ((fd = open(fname, O_RDWR | O_CREAT, 0700)) == -1) {
-			tst_brkm(TBROK | TTERRNO, cleanup,
-				 "open(%s, O_RDWR|O_CREAT,0700) failed", fname);
-		}
-		TEST(close(fd));
-
-		if (TEST_RETURN == -1) {
-			tst_resm(TFAIL | TTERRNO, "close(%s) failed", fname);
-		} else {
-			tst_resm(TPASS, "close(%s) returned %ld", fname,
-				 TEST_RETURN);
-		}
-
-		SAFE_UNLINK(cleanup, fname);
-	}
+#include <sys/types.h>
+#include "tst_test.h"

-	cleanup();
-	tst_exit();
-}
+#define FILENAME "close08_testfile"

-void setup(void)
+static void run(void)
 {
-
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
-
-	tst_tmpdir();
-
-	sprintf(fname, "tfile_%d", getpid());
+    int fd = SAFE_OPEN(FILENAME, O_RDWR | O_CREAT, 0700);
+    TST_EXP_PASS(close(fd));
 }

-void cleanup(void)
-{
-	tst_rmdir();
-
-}
+static struct tst_test test = {
+        .needs_tmpdir = 1,
+        .test_all = run,
+};
--
2.17.1


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

* [LTP] [PATCH 1/3 v2] syscalls/close: Convert close01 to the new API
  2021-04-26 12:52 ` [LTP] [PATCH 1/3 v2] syscalls/close: Convert close01 " Xie Ziyao
@ 2021-04-27 12:15   ` Cyril Hrubis
  2021-04-28  3:11     ` xieziyao
  0 siblings, 1 reply; 8+ messages in thread
From: Cyril Hrubis @ 2021-04-27 12:15 UTC (permalink / raw)
  To: ltp

Hi!
> 1. Cleanup and convert close01 to the new API;
> 2. Add SAFE_CLOSE() to the file descriptor.
> 
> Signed-off-by: Xie Ziyao <xieziyao@huawei.com>
> ---
> v1->v2:
> 1. Cleanup with TST_EXP_PASS();
> 2. Add SAFE_CLOSE() to the file descriptor for a pipe.
> 
>  testcases/kernel/syscalls/close/close01.c | 134 +++++++---------------
>  1 file changed, 40 insertions(+), 94 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/close/close01.c b/testcases/kernel/syscalls/close/close01.c
> index c734ff7d2..759c43af0 100644
> --- a/testcases/kernel/syscalls/close/close01.c
> +++ b/testcases/kernel/syscalls/close/close01.c
> @@ -1,124 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * Copyright (c) International Business Machines  Corp., 2001
> - *  07/2001 Ported by Wayne Boyer
> + * 07/2001 Ported by Wayne Boyer
> + */
> +
> +/*\
> + * [Description]
>   *
> - * 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.
> + * Basic test for the close() syscall.
>   *
> - * 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.
> + * [Description]
>   *
> - * 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
> - *	Test that closing a regular file and a pipe works correctly
> + * Test that closing a regular file and a pipe works correctly.
>   */

We ended up with two [Description] sections here, I guess that we should
remove the second occurence of '[Description]' here.

>  #include <stdio.h>
>  #include <errno.h>
>  #include <fcntl.h>
>  #include <sys/stat.h>
> -#include "test.h"
> -#include "safe_macros.h"
> +#include "tst_test.h"
> 
> -void cleanup(void);
> -void setup(void);
> +#define FILENAME "close01_testfile"
> 
> -char *TCID = "close01";
> -int TST_TOTAL = 2;
> +int fild, newfd, pipefildes[2];
> 
> -char fname[40] = "";
> +static void setup_file(void)
> +{
> +	newfd = SAFE_DUP(fild);
> +}
> 
> -int fild, newfd, pipefildes[2];
> +static void setup_pipe(void)
> +{
> +	SAFE_PIPE(pipefildes);
> +	SAFE_CLOSE(pipefildes[1]);
> +}
> 
>  struct test_case_t {
>  	int *fd;
>  	char *type;
> -} TC[] = {
> -	/* file descriptor for a regular file */
> -	{
> -	&newfd, "file"},
> -	    /* file descriptor for a pipe */
> -	{
> -	&pipefildes[0], "pipe"}
> +	void (*setupfunc) ();
> +} tc[] = {
> +	{&newfd, "file", setup_file},
> +	{&pipefildes[0], "pipe", setup_pipe}
>  };

Why don't we change the code so that the setup function returns the
filedescriptor? That would make the code much more straightforward.

> -int main(int ac, char **av)
> +static void run(unsigned int i)
>  {
> -
> -	int i;
> -	int lc;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> -
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -
> -		tst_count = 0;
> -
> -		if ((fild = creat(fname, 0777)) == -1)
> -			tst_brkm(TBROK | TERRNO, cleanup, "can't open file %s",
> -				 fname);
> -
> -		if ((newfd = dup(fild)) == -1)
> -			tst_brkm(TBROK | TERRNO, cleanup,
> -				 "can't dup the file des");
> -
> -		SAFE_PIPE(cleanup, pipefildes);
> -
> -		for (i = 0; i < TST_TOTAL; i++) {
> -
> -			TEST(close(*TC[i].fd));
> -
> -			if (TEST_RETURN == -1) {
> -				tst_resm(TFAIL, "call failed unexpectedly");
> -				continue;
> -			}
> -
> -			if (close(*TC[i].fd) == -1) {
> -				tst_resm(TPASS, "%s appears closed",
> -					 TC[i].type);
> -			} else {
> -				tst_resm(TFAIL, "%s close succeeded on"
> -					 "second attempt", TC[i].type);
> -			}
> -		}
> -
> -	}
> -
> -	cleanup();
> -	tst_exit();
> +	tc[i].setupfunc();
> +	TST_EXP_PASS(close(*tc[i].fd), "close a %s", tc[i].type);
>  }
> 
> -void setup(void)
> +static void setup(void)
>  {
> -	int mypid;
> -
> -	tst_sig(FORK, DEF_HANDLER, cleanup);
> -
>  	umask(0);

I'm pretty sure the umask() here is useless.


Otherwise it looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 2/3 v2] syscalls/close: Convert close02 to the new API
  2021-04-26 12:52 ` [LTP] [PATCH 2/3 v2] syscalls/close: Convert close02 " Xie Ziyao
@ 2021-04-27 12:20   ` Cyril Hrubis
  0 siblings, 0 replies; 8+ messages in thread
From: Cyril Hrubis @ 2021-04-27 12:20 UTC (permalink / raw)
  To: ltp

Hi!
Pushed with a minor change, thanks.

> +/*\
> + * [Description]
> + *
> + * Basic test for the close() syscall.

This sentence does not add any more information, so I've removed it and
kept the second sentence below as a description instead.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 3/3 v2] syscalls/close: Convert close08 to the new API
  2021-04-26 12:52 ` [LTP] [PATCH 3/3 v2] syscalls/close: Convert close08 " Xie Ziyao
@ 2021-04-27 12:23   ` Cyril Hrubis
  0 siblings, 0 replies; 8+ messages in thread
From: Cyril Hrubis @ 2021-04-27 12:23 UTC (permalink / raw)
  To: ltp

Hi!
Isn't this test just the same as close01.c?

I think that we can just remove close08.c since it does not add any more
coverage over close01.

Also we can add all kind of other file descriptor to the close01.c test
as well. We should add at least a few different socket fds there as well.

-- 
Cyril Hrubis
chrubis@suse.cz

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

* [LTP] [PATCH 1/3 v2] syscalls/close: Convert close01 to the new API
  2021-04-27 12:15   ` Cyril Hrubis
@ 2021-04-28  3:11     ` xieziyao
  0 siblings, 0 replies; 8+ messages in thread
From: xieziyao @ 2021-04-28  3:11 UTC (permalink / raw)
  To: ltp

Hi, Cyril,

I will modify it based on your comments. In addition, changes for close08 will be incorporated into this case.

Thanks for your review.

Best Regards,
Ziyao

-----Original Message-----
From: Cyril Hrubis [mailto:chrubis@suse.cz] 
Sent: Tuesday, April 27, 2021 8:15 PM
To: xieziyao <xieziyao@huawei.com>
Cc: ltp@lists.linux.it
Subject: Re: [LTP] [PATCH 1/3 v2] syscalls/close: Convert close01 to the new API

Hi!
> 1. Cleanup and convert close01 to the new API; 2. Add SAFE_CLOSE() to 
> the file descriptor.
> 
> Signed-off-by: Xie Ziyao <xieziyao@huawei.com>
> ---
> v1->v2:
> 1. Cleanup with TST_EXP_PASS();
> 2. Add SAFE_CLOSE() to the file descriptor for a pipe.
> 
>  testcases/kernel/syscalls/close/close01.c | 134 
> +++++++---------------
>  1 file changed, 40 insertions(+), 94 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/close/close01.c 
> b/testcases/kernel/syscalls/close/close01.c
> index c734ff7d2..759c43af0 100644
> --- a/testcases/kernel/syscalls/close/close01.c
> +++ b/testcases/kernel/syscalls/close/close01.c
> @@ -1,124 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * Copyright (c) International Business Machines  Corp., 2001
> - *  07/2001 Ported by Wayne Boyer
> + * 07/2001 Ported by Wayne Boyer
> + */
> +
> +/*\
> + * [Description]
>   *
> - * 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.
> + * Basic test for the close() syscall.
>   *
> - * 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.
> + * [Description]
>   *
> - * 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
> - *	Test that closing a regular file and a pipe works correctly
> + * Test that closing a regular file and a pipe works correctly.
>   */

We ended up with two [Description] sections here, I guess that we should remove the second occurence of '[Description]' here.

>  #include <stdio.h>
>  #include <errno.h>
>  #include <fcntl.h>
>  #include <sys/stat.h>
> -#include "test.h"
> -#include "safe_macros.h"
> +#include "tst_test.h"
> 
> -void cleanup(void);
> -void setup(void);
> +#define FILENAME "close01_testfile"
> 
> -char *TCID = "close01";
> -int TST_TOTAL = 2;
> +int fild, newfd, pipefildes[2];
> 
> -char fname[40] = "";
> +static void setup_file(void)
> +{
> +	newfd = SAFE_DUP(fild);
> +}
> 
> -int fild, newfd, pipefildes[2];
> +static void setup_pipe(void)
> +{
> +	SAFE_PIPE(pipefildes);
> +	SAFE_CLOSE(pipefildes[1]);
> +}
> 
>  struct test_case_t {
>  	int *fd;
>  	char *type;
> -} TC[] = {
> -	/* file descriptor for a regular file */
> -	{
> -	&newfd, "file"},
> -	    /* file descriptor for a pipe */
> -	{
> -	&pipefildes[0], "pipe"}
> +	void (*setupfunc) ();
> +} tc[] = {
> +	{&newfd, "file", setup_file},
> +	{&pipefildes[0], "pipe", setup_pipe}
>  };

Why don't we change the code so that the setup function returns the filedescriptor? That would make the code much more straightforward.

> -int main(int ac, char **av)
> +static void run(unsigned int i)
>  {
> -
> -	int i;
> -	int lc;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> -
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -
> -		tst_count = 0;
> -
> -		if ((fild = creat(fname, 0777)) == -1)
> -			tst_brkm(TBROK | TERRNO, cleanup, "can't open file %s",
> -				 fname);
> -
> -		if ((newfd = dup(fild)) == -1)
> -			tst_brkm(TBROK | TERRNO, cleanup,
> -				 "can't dup the file des");
> -
> -		SAFE_PIPE(cleanup, pipefildes);
> -
> -		for (i = 0; i < TST_TOTAL; i++) {
> -
> -			TEST(close(*TC[i].fd));
> -
> -			if (TEST_RETURN == -1) {
> -				tst_resm(TFAIL, "call failed unexpectedly");
> -				continue;
> -			}
> -
> -			if (close(*TC[i].fd) == -1) {
> -				tst_resm(TPASS, "%s appears closed",
> -					 TC[i].type);
> -			} else {
> -				tst_resm(TFAIL, "%s close succeeded on"
> -					 "second attempt", TC[i].type);
> -			}
> -		}
> -
> -	}
> -
> -	cleanup();
> -	tst_exit();
> +	tc[i].setupfunc();
> +	TST_EXP_PASS(close(*tc[i].fd), "close a %s", tc[i].type);
>  }
> 
> -void setup(void)
> +static void setup(void)
>  {
> -	int mypid;
> -
> -	tst_sig(FORK, DEF_HANDLER, cleanup);
> -
>  	umask(0);

I'm pretty sure the umask() here is useless.


Otherwise it looks good.

--
Cyril Hrubis
chrubis@suse.cz

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

end of thread, other threads:[~2021-04-28  3:11 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-04-26 12:52 [LTP] [PATCH 0/3 v2] syscalls/close: Convert close{01, 02, 08} to the new API Xie Ziyao
2021-04-26 12:52 ` [LTP] [PATCH 1/3 v2] syscalls/close: Convert close01 " Xie Ziyao
2021-04-27 12:15   ` Cyril Hrubis
2021-04-28  3:11     ` xieziyao
2021-04-26 12:52 ` [LTP] [PATCH 2/3 v2] syscalls/close: Convert close02 " Xie Ziyao
2021-04-27 12:20   ` Cyril Hrubis
2021-04-26 12:52 ` [LTP] [PATCH 3/3 v2] syscalls/close: Convert close08 " Xie Ziyao
2021-04-27 12:23   ` Cyril Hrubis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox