[LTP] [PATCH 0/2] signalfd: Improve testsuite for signalfd

[LTP] [PATCH 0/2] signalfd: Improve testsuite for signalfd

[Ma Xinjian via ltp]

Currently, testsuite for signalfd is written in old API, and only
contains case for normal situation.

So I refactor the existed case with new API, and add case for negative
situations.

Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com>

signalfd01: Refactor old case with new API
signalfd: Add negative tests
---
 runtest/syscalls                                |   1 +
 testcases/kernel/syscalls/signalfd/.gitignore   |   1 +
 testcases/kernel/syscalls/signalfd/signalfd01.c | 350 +++++++-----------------
 testcases/kernel/syscalls/signalfd/signalfd02.c | 115 ++++++++
 4 files changed, 213 insertions(+), 254 deletions(-)
--- 
2.39.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/2] signalfd01: Refactor old case with new API

[Ma Xinjian via ltp]

signalfd01 is written in old API and some code seems unreachable. It needs
to be refactored with new API to simplify the logic.

Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com>
---
 .../kernel/syscalls/signalfd/signalfd01.c     | 350 +++++-------------
 1 file changed, 96 insertions(+), 254 deletions(-)

diff --git a/testcases/kernel/syscalls/signalfd/signalfd01.c b/testcases/kernel/syscalls/signalfd/signalfd01.c
index 8fb16800f..f3c305194 100644
--- a/testcases/kernel/syscalls/signalfd/signalfd01.c
+++ b/testcases/kernel/syscalls/signalfd/signalfd01.c
@@ -1,302 +1,144 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *
- *   Copyright (c) Red Hat Inc., 2008
- *
- *   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) Red Hat Inc., 2008
+ * Copyright (c) Linux Test Project, 2006-2024
  */
 
-/*
- * NAME
- *	signalfd01.c
+/*\
+ * [Description]
  *
- * DESCRIPTION
- *	Check signalfd can receive signals
+ * Verify that signalfd() can receive signals
  *
- * USAGE
- * 	signalfd01
- *
- * HISTORY
- *	9/2008 Initial version by Masatake YAMATO <yamato@redhat.com>
- *
- * RESTRICTIONS
- * 	None
  */
-#define _GNU_SOURCE
-
-#include "config.h"
 
-#include "test.h"
+#define _GNU_SOURCE
 
-#include <errno.h>
 #include <signal.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <inttypes.h>
-#include "ltp_signal.h"
-
-TCID_DEFINE(signalfd01);
-int TST_TOTAL = 1;
+#include "tst_test.h"
+#include "config.h"
 
 #ifndef HAVE_SIGNALFD
-#define  USE_STUB
+#define USE_STUB
 #endif
 
-#if defined HAVE_SYS_SIGNALFD_H
+#ifdef HAVE_SYS_SIGNALFD_H
 #include <sys/signalfd.h>
 #elif defined HAVE_LINUX_SIGNALFD_H
 #include <linux/signalfd.h>
 #define USE_OWNIMPL
 #else
-#define  USE_STUB
-#endif
+#define USE_STUB
+#endif /* HAVE_SYS_SIGNALFD_H */
 
 #ifndef HAVE_STRUCT_SIGNALFD_SIGINFO_SSI_SIGNO
 #define USE_STUB
-#endif
+#endif /* HAVE_STRUCT_SIGNALFD_SIGINFO_SSI_SIGNO */
 
 #ifdef USE_STUB
-int main(void)
-{
-	tst_brkm(TCONF, NULL, "System doesn't support execution of the test");
-}
-
-#else
+TST_TEST_TCONF("This system does not support signalfd()");
+#else /* USE_STUB */
 #if defined USE_OWNIMPL
 #include "lapi/syscalls.h"
-int signalfd(int fd, const sigset_t * mask, int flags)
+int signalfd(int fd, const sigset_t *mask, int flags)
 {
-	/* Taken from GLIBC. */
-	return tst_syscall(__NR_signalfd, fd, mask, SIGSETSIZE);
+	return tst_syscall(__NR_signalfd, fd, mask, flags);
 }
-#endif
+#endif /* USE_OWNIMPL */
 
-void cleanup(void);
-void setup(void);
+static int fd_signal = -1;
+static sigset_t mask1;
+static sigset_t mask2;
 
-int do_test1(uint32_t sig)
+static int check_signal(int fd, uint32_t signal)
 {
-	int sfd_for_next;
-	int sfd;
-	sigset_t mask;
-	pid_t pid;
-	struct signalfd_siginfo fdsi;
-	ssize_t s;
-
-	sigemptyset(&mask);
-	sigaddset(&mask, sig);
-	if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) {
-		tst_brkm(TBROK, cleanup,
-			 "sigprocmask() Failed: errno=%d : %s",
-			 errno, strerror(errno));
-	}
-
-	TEST(signalfd(-1, &mask, 0));
-
-	if ((sfd = TEST_RETURN) == -1) {
-		tst_resm(TFAIL,
-			 "signalfd() Failed, errno=%d : %s",
-			 TEST_ERRNO, strerror(TEST_ERRNO));
-		sfd_for_next = -1;
-		return sfd_for_next;
-
-	} else {
-		tst_resm(TPASS, "signalfd is created successfully");
-		sfd_for_next = sfd;
-		goto out;
-	}
-
-	if (fcntl(sfd, F_SETFL, O_NONBLOCK) == -1) {
-		close(sfd);
-		tst_brkm(TBROK, cleanup,
-			 "setting signalfd nonblocking mode failed: errno=%d : %s",
-			 errno, strerror(errno));
-	}
-
-	pid = getpid();
-	if (kill(pid, sig) == -1) {
-		close(sfd);
-		tst_brkm(TBROK, cleanup,
-			 "kill(self, %s) failed: errno=%d : %s",
-			 strsignal(sig), errno, strerror(errno));
-	}
-
-	s = read(sfd, &fdsi, sizeof(struct signalfd_siginfo));
-	if ((s > 0) && (s != sizeof(struct signalfd_siginfo))) {
-		tst_resm(TFAIL,
-			 "getting incomplete signalfd_siginfo data: "
-			 "actual-size=%zd, expected-size=%zu",
-			 s, sizeof(struct signalfd_siginfo));
-		sfd_for_next = -1;
-		close(sfd);
-		goto out;
-	} else if (s < 0) {
-		if (errno == EAGAIN) {
-			tst_resm(TFAIL,
-				 "signalfd_siginfo data is not delivered yet");
-			sfd_for_next = -1;
-			close(sfd);
-			goto out;
-		} else {
-			close(sfd);
-			tst_brkm(TBROK, cleanup,
-				 "read signalfd_siginfo data failed: errno=%d : %s",
-				 errno, strerror(errno));
-		}
-	} else if (s == 0) {
-		tst_resm(TFAIL, "got EOF unexpectedly");
-		sfd_for_next = -1;
-		close(sfd);
-		goto out;
-	}
-
-	if (fdsi.ssi_signo == sig) {
-		tst_resm(TPASS, "got expected signal");
-		sfd_for_next = sfd;
-		goto out;
+	pid_t pid = getpid();
+	ssize_t bytes;
+	struct signalfd_siginfo siginfo;
+	int ret = -1;
+
+	SAFE_KILL(pid, signal);
+	bytes = read(fd, &siginfo, sizeof(siginfo));
+	if (bytes < 0) {
+		if (errno == EAGAIN)
+			tst_res(TFAIL | TERRNO, "%s",
+				"signal has not been received yet");
+		else
+			tst_res(TFAIL | TERRNO, "%s",
+				"error occured during reading signal");
 	} else {
-		tst_resm(TFAIL, "got unexpected signal: signal=%d : %s",
-			 fdsi.ssi_signo, strsignal(fdsi.ssi_signo));
-		sfd_for_next = -1;
-		close(sfd);
-		goto out;
+		if (bytes != sizeof(siginfo))
+			tst_res(TFAIL | TERRNO, "%s",
+				"signal received is not completed");
+		else
+			if (siginfo.ssi_signo != signal)
+				tst_res(TFAIL | TERRNO,
+					"unexpected signal <%s>(%d) received",
+					strsignal(siginfo.ssi_signo),
+					siginfo.ssi_signo);
+			else
+				ret = 0;
 	}
-
-out:
-	return sfd_for_next;
+	return ret;
 }
 
-void do_test2(int fd, uint32_t sig)
+static void setup(void)
 {
-	int sfd;
-	sigset_t mask;
-	pid_t pid;
-	struct signalfd_siginfo fdsi;
-	ssize_t s;
-
-	sigemptyset(&mask);
-	sigaddset(&mask, sig);
-	if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) {
-		close(fd);
-		tst_brkm(TBROK, cleanup,
-			 "sigprocmask() Failed: errno=%d : %s",
-			 errno, strerror(errno));
-	}
-
-	TEST(signalfd(fd, &mask, 0));
-
-	if ((sfd = TEST_RETURN) == -1) {
-		tst_resm(TFAIL,
-			 "reassignment the file descriptor by signalfd() failed, errno=%d : %s",
-			 TEST_ERRNO, strerror(TEST_ERRNO));
-		return;
-	} else if (sfd != fd) {
-		tst_resm(TFAIL,
-			 "different fd is returned in reassignment: expected-fd=%d, actual-fd=%d",
-			 fd, sfd);
-		close(sfd);
-		return;
-
-	} else {
-		tst_resm(TPASS, "signalfd is successfully reassigned");
-		goto out;
-	}
-
-	pid = getpid();
-	if (kill(pid, sig) == -1) {
-		close(sfd);
-		tst_brkm(TBROK, cleanup,
-			 "kill(self, %s) failed: errno=%d : %s",
-			 strsignal(sig), errno, strerror(errno));
-	}
-
-	s = read(sfd, &fdsi, sizeof(struct signalfd_siginfo));
-	if ((s > 0) && (s != sizeof(struct signalfd_siginfo))) {
-		tst_resm(TFAIL,
-			 "getting incomplete signalfd_siginfo data: "
-			 "actual-size=%zd, expected-size= %zu",
-			 s, sizeof(struct signalfd_siginfo));
-		goto out;
-	} else if (s < 0) {
-		if (errno == EAGAIN) {
-			tst_resm(TFAIL,
-				 "signalfd_siginfo data is not delivered yet");
-			goto out;
-		} else {
-			close(sfd);
-			tst_brkm(TBROK, cleanup,
-				 "read signalfd_siginfo data failed: errno=%d : %s",
-				 errno, strerror(errno));
-		}
-	} else if (s == 0) {
-		tst_resm(TFAIL, "got EOF unexpectedly");
-		goto out;
-	}
-
-	if (fdsi.ssi_signo == sig) {
-		tst_resm(TPASS, "got expected signal");
-		goto out;
-	} else {
-		tst_resm(TFAIL, "got unexpected signal: signal=%d : %s",
-			 fdsi.ssi_signo, strsignal(fdsi.ssi_signo));
-		goto out;
-	}
-
-out:
-	return;
+	SAFE_SIGEMPTYSET(&mask1);
+	SAFE_SIGADDSET(&mask1, SIGUSR1);
+	SAFE_SIGPROCMASK(SIG_BLOCK, &mask1, NULL);
+	SAFE_SIGEMPTYSET(&mask2);
+	SAFE_SIGADDSET(&mask2, SIGUSR2);
+	SAFE_SIGPROCMASK(SIG_BLOCK, &mask2, NULL);
 }
 
-int main(int argc, char **argv)
+static void cleanup(void)
 {
-	int lc;
-	int sfd;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	setup();
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		sfd = do_test1(SIGUSR1);
-		if (sfd < 0)
-			continue;
+	if (fd_signal > 0)
+		SAFE_CLOSE(fd_signal);
+}
 
-		do_test2(sfd, SIGUSR2);
-		close(sfd);
+static void do_create(void)
+{
+	TST_EXP_FD_SILENT(signalfd(fd_signal, &mask1, 0),
+		"%s", "signalfd() create fd");
+	if (TST_RET > 0) {
+		fd_signal = TST_RET;
+		if (!check_signal(fd_signal, SIGUSR1))
+			tst_res(TPASS, "%s",
+				"signalfd() created fd successfully");
 	}
-
-	cleanup();
-
-	tst_exit();
 }
 
-/*
- * setup() - performs all the ONE TIME setup for this test.
- */
-void setup(void)
+static void do_reassign(void)
 {
-
-	TEST_PAUSE;
+	if (fd_signal < 0)
+		tst_brk(TBROK, "%s", "Invalid fd. Skip reassign");
+
+	TST_EXP_FD_SILENT(signalfd(fd_signal, &mask2, 0), "%s",
+		"signalfd() reassign fd");
+	if (TST_RET > 0) {
+		if (TST_RET == fd_signal) {
+			if (!check_signal(fd_signal, SIGUSR2))
+				tst_res(TPASS, "%s",
+					"signalfd() reassigned fd "
+					"successfully");
+		} else {
+			tst_res(TFAIL | TERRNO, "%s",
+				"fd changed unexpectedly during signalfd()");
+		}
+	}
 }
 
-/*
- * cleanup() - performs all the ONE TIME cleanup for this test at completion
- * 	       or premature exit.
- */
-void cleanup(void)
+static void verify_signalfd(void)
 {
-
+	do_create();
+	do_reassign();
 }
 
-#endif
+static struct tst_test test = {
+	.test_all = verify_signalfd,
+	.setup = setup,
+	.cleanup = cleanup,
+};
+
+#endif /* USE_STUB */
-- 
2.39.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] signalfd01: Refactor old case with new API

[Andrea Cervesato via ltp]

Hi!

On 5/28/24 12:32, Ma Xinjian via ltp wrote:
> signalfd01 is written in old API and some code seems unreachable. It needs
> to be refactored with new API to simplify the logic.
>
> Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com>
> ---
>   .../kernel/syscalls/signalfd/signalfd01.c     | 350 +++++-------------
>   1 file changed, 96 insertions(+), 254 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/signalfd/signalfd01.c b/testcases/kernel/syscalls/signalfd/signalfd01.c
> index 8fb16800f..f3c305194 100644
> --- a/testcases/kernel/syscalls/signalfd/signalfd01.c
> +++ b/testcases/kernel/syscalls/signalfd/signalfd01.c
> @@ -1,302 +1,144 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>   /*
> - *
> - *   Copyright (c) Red Hat Inc., 2008
> - *
> - *   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) Red Hat Inc., 2008
> + * Copyright (c) Linux Test Project, 2006-2024
>    */
>   
> -/*
> - * NAME
> - *	signalfd01.c
> +/*\
> + * [Description]
>    *
> - * DESCRIPTION
> - *	Check signalfd can receive signals
> + * Verify that signalfd() can receive signals
This description is a bit too simple and we can describe what test does 
in particular.
>    *
> - * USAGE
> - * 	signalfd01
> - *
> - * HISTORY
> - *	9/2008 Initial version by Masatake YAMATO <yamato@redhat.com>
> - *
> - * RESTRICTIONS
> - * 	None
>    */
> -#define _GNU_SOURCE
> -
> -#include "config.h"
>   
> -#include "test.h"
> +#define _GNU_SOURCE
_GNU_SOURCE is not needed, since signalfd() is a standard feature and no 
other parts of the test require nonstandard features.
>   
> -#include <errno.h>
>   #include <signal.h>
> -#include <unistd.h>
> -#include <fcntl.h>
> -#include <inttypes.h>
> -#include "ltp_signal.h"
> -
> -TCID_DEFINE(signalfd01);
> -int TST_TOTAL = 1;
> +#include "tst_test.h"
> +#include "config.h"
>   
>   #ifndef HAVE_SIGNALFD
> -#define  USE_STUB
> +#define USE_STUB
signalfd() is present since Linux 2.6.22 and glibc 2.8, which are below 
our minimum supported versions.
For this reason, we don't need any stub function definition.

Please take a look at 
https://linux-test-project.readthedocs.io/en/latest/users/supported_systems.html
>   #endif
>   
> -#if defined HAVE_SYS_SIGNALFD_H
> +#ifdef HAVE_SYS_SIGNALFD_H
No need for this check, for the same reason above. Maybe it can be 
removed from autotools configuration as well.
>   #include <sys/signalfd.h>
>   #elif defined HAVE_LINUX_SIGNALFD_H

not needed for the same reason above.
>   #include <linux/signalfd.h>
>   #define USE_OWNIMPL
not needed for the same reason above.
>   #else
> -#define  USE_STUB
> -#endif
> +#define USE_STUB
> +#endif /* HAVE_SYS_SIGNALFD_H */
>   
>   #ifndef HAVE_STRUCT_SIGNALFD_SIGINFO_SSI_SIGNO

not needed for the same reason above.
>   #define USE_STUB
> -#endif
> +#endif /* HAVE_STRUCT_SIGNALFD_SIGINFO_SSI_SIGNO */
>   
>   #ifdef USE_STUB
> -int main(void)
> -{
> -	tst_brkm(TCONF, NULL, "System doesn't support execution of the test");
> -}
> -
> -#else
> +TST_TEST_TCONF("This system does not support signalfd()");

not needed for the same reason above.
> +#else /* USE_STUB */
>   #if defined USE_OWNIMPL
>   #include "lapi/syscalls.h"
> -int signalfd(int fd, const sigset_t * mask, int flags)
> +int signalfd(int fd, const sigset_t *mask, int flags)
>   {
> -	/* Taken from GLIBC. */
> -	return tst_syscall(__NR_signalfd, fd, mask, SIGSETSIZE);
> +	return tst_syscall(__NR_signalfd, fd, mask, flags);
>   }
> -#endif
> +#endif /* USE_OWNIMPL */
>   
> -void cleanup(void);
> -void setup(void);
> +static int fd_signal = -1;
> +static sigset_t mask1;
> +static sigset_t mask2;
>   
> -int do_test1(uint32_t sig)
> +static int check_signal(int fd, uint32_t signal)
>   {
> -	int sfd_for_next;
> -	int sfd;
> -	sigset_t mask;
> -	pid_t pid;
> -	struct signalfd_siginfo fdsi;
> -	ssize_t s;
> -
> -	sigemptyset(&mask);
> -	sigaddset(&mask, sig);
> -	if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) {
> -		tst_brkm(TBROK, cleanup,
> -			 "sigprocmask() Failed: errno=%d : %s",
> -			 errno, strerror(errno));
> -	}
> -
> -	TEST(signalfd(-1, &mask, 0));
> -
> -	if ((sfd = TEST_RETURN) == -1) {
> -		tst_resm(TFAIL,
> -			 "signalfd() Failed, errno=%d : %s",
> -			 TEST_ERRNO, strerror(TEST_ERRNO));
> -		sfd_for_next = -1;
> -		return sfd_for_next;
> -
> -	} else {
> -		tst_resm(TPASS, "signalfd is created successfully");
> -		sfd_for_next = sfd;
> -		goto out;
> -	}
> -
> -	if (fcntl(sfd, F_SETFL, O_NONBLOCK) == -1) {
> -		close(sfd);
> -		tst_brkm(TBROK, cleanup,
> -			 "setting signalfd nonblocking mode failed: errno=%d : %s",
> -			 errno, strerror(errno));
> -	}
> -
> -	pid = getpid();
> -	if (kill(pid, sig) == -1) {
> -		close(sfd);
> -		tst_brkm(TBROK, cleanup,
> -			 "kill(self, %s) failed: errno=%d : %s",
> -			 strsignal(sig), errno, strerror(errno));
> -	}
> -
> -	s = read(sfd, &fdsi, sizeof(struct signalfd_siginfo));
> -	if ((s > 0) && (s != sizeof(struct signalfd_siginfo))) {
> -		tst_resm(TFAIL,
> -			 "getting incomplete signalfd_siginfo data: "
> -			 "actual-size=%zd, expected-size=%zu",
> -			 s, sizeof(struct signalfd_siginfo));
> -		sfd_for_next = -1;
> -		close(sfd);
> -		goto out;
> -	} else if (s < 0) {
> -		if (errno == EAGAIN) {
> -			tst_resm(TFAIL,
> -				 "signalfd_siginfo data is not delivered yet");
> -			sfd_for_next = -1;
> -			close(sfd);
> -			goto out;
> -		} else {
> -			close(sfd);
> -			tst_brkm(TBROK, cleanup,
> -				 "read signalfd_siginfo data failed: errno=%d : %s",
> -				 errno, strerror(errno));
> -		}
> -	} else if (s == 0) {
> -		tst_resm(TFAIL, "got EOF unexpectedly");
> -		sfd_for_next = -1;
> -		close(sfd);
> -		goto out;
> -	}
> -
> -	if (fdsi.ssi_signo == sig) {
> -		tst_resm(TPASS, "got expected signal");
> -		sfd_for_next = sfd;
> -		goto out;
> +	pid_t pid = getpid();
> +	ssize_t bytes;
> +	struct signalfd_siginfo siginfo;
> +	int ret = -1;
> +
> +	SAFE_KILL(pid, signal);
> +	bytes = read(fd, &siginfo, sizeof(siginfo));
We can just use SAFE_READ()
> +	if (bytes < 0) {
> +		if (errno == EAGAIN)
> +			tst_res(TFAIL | TERRNO, "%s",
> +				"signal has not been received yet");
> +		else
> +			tst_res(TFAIL | TERRNO, "%s",
> +				"error occured during reading signal");
>   	} else {
> -		tst_resm(TFAIL, "got unexpected signal: signal=%d : %s",
> -			 fdsi.ssi_signo, strsignal(fdsi.ssi_signo));
> -		sfd_for_next = -1;
> -		close(sfd);
> -		goto out;
> +		if (bytes != sizeof(siginfo))
Better to use TST_EXP_EQ_LI()
> +			tst_res(TFAIL | TERRNO, "%s",
> +				"signal received is not completed");
> +		else
> +			if (siginfo.ssi_signo != signal)
Better to use TST_EXP_EQ_LI()
> +				tst_res(TFAIL | TERRNO,
> +					"unexpected signal <%s>(%d) received",
> +					strsignal(siginfo.ssi_signo),
> +					siginfo.ssi_signo);
> +			else
> +				ret = 0;
>   	}

The whole function can be simplified as following:

     bytes = SAFE_READ(0, fd, &siginfo, sizeof(siginfo));
     TST_EXP_EQ_LI(bytes, sizeof(siginfo));
     TST_EXP_EQ_LI(siginfo.ssi_signo, signal);

> -
> -out:
> -	return sfd_for_next;
> +	return ret;
There's no need to return any value here, since we can directly handle 
TPASS.
>   }
>   
> -void do_test2(int fd, uint32_t sig)
> +static void setup(void)
>   {
> -	int sfd;
> -	sigset_t mask;
> -	pid_t pid;
> -	struct signalfd_siginfo fdsi;
> -	ssize_t s;
> -
> -	sigemptyset(&mask);
> -	sigaddset(&mask, sig);
> -	if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) {
> -		close(fd);
> -		tst_brkm(TBROK, cleanup,
> -			 "sigprocmask() Failed: errno=%d : %s",
> -			 errno, strerror(errno));
> -	}
> -
> -	TEST(signalfd(fd, &mask, 0));
> -
> -	if ((sfd = TEST_RETURN) == -1) {
> -		tst_resm(TFAIL,
> -			 "reassignment the file descriptor by signalfd() failed, errno=%d : %s",
> -			 TEST_ERRNO, strerror(TEST_ERRNO));
> -		return;
> -	} else if (sfd != fd) {
> -		tst_resm(TFAIL,
> -			 "different fd is returned in reassignment: expected-fd=%d, actual-fd=%d",
> -			 fd, sfd);
> -		close(sfd);
> -		return;
> -
> -	} else {
> -		tst_resm(TPASS, "signalfd is successfully reassigned");
> -		goto out;
> -	}
> -
> -	pid = getpid();
> -	if (kill(pid, sig) == -1) {
> -		close(sfd);
> -		tst_brkm(TBROK, cleanup,
> -			 "kill(self, %s) failed: errno=%d : %s",
> -			 strsignal(sig), errno, strerror(errno));
> -	}
> -
> -	s = read(sfd, &fdsi, sizeof(struct signalfd_siginfo));
> -	if ((s > 0) && (s != sizeof(struct signalfd_siginfo))) {
> -		tst_resm(TFAIL,
> -			 "getting incomplete signalfd_siginfo data: "
> -			 "actual-size=%zd, expected-size= %zu",
> -			 s, sizeof(struct signalfd_siginfo));
> -		goto out;
> -	} else if (s < 0) {
> -		if (errno == EAGAIN) {
> -			tst_resm(TFAIL,
> -				 "signalfd_siginfo data is not delivered yet");
> -			goto out;
> -		} else {
> -			close(sfd);
> -			tst_brkm(TBROK, cleanup,
> -				 "read signalfd_siginfo data failed: errno=%d : %s",
> -				 errno, strerror(errno));
> -		}
> -	} else if (s == 0) {
> -		tst_resm(TFAIL, "got EOF unexpectedly");
> -		goto out;
> -	}
> -
> -	if (fdsi.ssi_signo == sig) {
> -		tst_resm(TPASS, "got expected signal");
> -		goto out;
> -	} else {
> -		tst_resm(TFAIL, "got unexpected signal: signal=%d : %s",
> -			 fdsi.ssi_signo, strsignal(fdsi.ssi_signo));
> -		goto out;
> -	}
> -
> -out:
> -	return;
> +	SAFE_SIGEMPTYSET(&mask1);
> +	SAFE_SIGADDSET(&mask1, SIGUSR1);
> +	SAFE_SIGPROCMASK(SIG_BLOCK, &mask1, NULL);
> +	SAFE_SIGEMPTYSET(&mask2);
> +	SAFE_SIGADDSET(&mask2, SIGUSR2);
> +	SAFE_SIGPROCMASK(SIG_BLOCK, &mask2, NULL);
>   }
>   
> -int main(int argc, char **argv)
> +static void cleanup(void)
>   {
> -	int lc;
> -	int sfd;
> -
> -	tst_parse_opts(argc, argv, NULL, NULL);
> -
> -	setup();
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		tst_count = 0;
> -
> -		sfd = do_test1(SIGUSR1);
> -		if (sfd < 0)
> -			continue;
> +	if (fd_signal > 0)
> +		SAFE_CLOSE(fd_signal);
> +}
>   
> -		do_test2(sfd, SIGUSR2);
> -		close(sfd);
> +static void do_create(void)
> +{
> +	TST_EXP_FD_SILENT(signalfd(fd_signal, &mask1, 0),
> +		"%s", "signalfd() create fd");
We want to test signalfd(), so we want to use TST_EXP_FD() instead.
> +	if (TST_RET > 0) {
> +		fd_signal = TST_RET;
> +		if (!check_signal(fd_signal, SIGUSR1))
> +			tst_res(TPASS, "%s",
> +				"signalfd() created fd successfully");

Better to handle TPASS inside check_signal()
In general, this whole test part would look better like this:

     TST_EXP_FD(signalfd(fd_signal, &mask1, 0));
     if (TST_RET == -1)
         return;

     fd_signal = TST_RET;
     check_signal(fd_signal, SIGUSR1);

>   	}
> -
> -	cleanup();
> -
> -	tst_exit();
>   }
>   
> -/*
> - * setup() - performs all the ONE TIME setup for this test.
> - */
> -void setup(void)
> +static void do_reassign(void)
>   {
> -
> -	TEST_PAUSE;
> +	if (fd_signal < 0)
> +		tst_brk(TBROK, "%s", "Invalid fd. Skip reassign");
> +
> +	TST_EXP_FD_SILENT(signalfd(fd_signal, &mask2, 0), "%s",
> +		"signalfd() reassign fd");
We want to test signalfd(), so we want to use TST_EXP_FD() instead.
> +	if (TST_RET > 0) {
> +		if (TST_RET == fd_signal) {
> +			if (!check_signal(fd_signal, SIGUSR2))
> +				tst_res(TPASS, "%s",
> +					"signalfd() reassigned fd "
> +					"successfully");
Better to handle TPASS inside check_signal()
> +		} else {
> +			tst_res(TFAIL | TERRNO, "%s",
> +				"fd changed unexpectedly during signalfd()");

No need to handle errors if we use TST_EXP_FD()

Same as before, it would look much better like this:


     TST_EXP_FD(signalfd(fd_signal, &mask2, 0), "%s", "signalfd() 
reassign fd");
     if (TST_RET == -1)
         return;

     TST_EXP_EQ_LI(TST_RET, fd_signal);
     check_signal(fd_signal, SIGUSR2);

> +		}
> +	}
>   }
>   
> -/*
> - * cleanup() - performs all the ONE TIME cleanup for this test at completion
> - * 	       or premature exit.
> - */
> -void cleanup(void)
> +static void verify_signalfd(void)
>   {
> -
> +	do_create();
> +	do_reassign();
No need to use 2 functions for this. They can be merged together
>   }
>   
> -#endif
> +static struct tst_test test = {
> +	.test_all = verify_signalfd,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +};
> +
> +#endif /* USE_STUB */

Andrea


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 1/2] signalfd01: Refactor old case with new API

[Ma Xinjian via ltp]

signalfd01 is written in old API and some code seems unreachable. It needs
to be refactored with new API to simplify the logic.

Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com>
---
 .../kernel/syscalls/signalfd/signalfd01.c     | 327 +++---------------
 1 file changed, 49 insertions(+), 278 deletions(-)

diff --git a/testcases/kernel/syscalls/signalfd/signalfd01.c b/testcases/kernel/syscalls/signalfd/signalfd01.c
index 8fb16800f..f0ef4a378 100644
--- a/testcases/kernel/syscalls/signalfd/signalfd01.c
+++ b/testcases/kernel/syscalls/signalfd/signalfd01.c
@@ -1,302 +1,73 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *
- *   Copyright (c) Red Hat Inc., 2008
- *
- *   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) Red Hat Inc., 2008
+ * Copyright (c) Linux Test Project, 2006-2024
  */
 
-/*
- * NAME
- *	signalfd01.c
- *
- * DESCRIPTION
- *	Check signalfd can receive signals
- *
- * USAGE
- * 	signalfd01
+/*\
+ * [Description]
  *
- * HISTORY
- *	9/2008 Initial version by Masatake YAMATO <yamato@redhat.com>
+ * Verify that signalfd() works as expected.
+ * 1. signalfd() can create fd, and fd can receive signal.
+ * 2. signalfd() can reassign fd, and fd can receive signal.
  *
- * RESTRICTIONS
- * 	None
  */
-#define _GNU_SOURCE
 
-#include "config.h"
-
-#include "test.h"
-
-#include <errno.h>
-#include <signal.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <inttypes.h>
-#include "ltp_signal.h"
-
-TCID_DEFINE(signalfd01);
-int TST_TOTAL = 1;
-
-#ifndef HAVE_SIGNALFD
-#define  USE_STUB
-#endif
-
-#if defined HAVE_SYS_SIGNALFD_H
 #include <sys/signalfd.h>
-#elif defined HAVE_LINUX_SIGNALFD_H
-#include <linux/signalfd.h>
-#define USE_OWNIMPL
-#else
-#define  USE_STUB
-#endif
+#include "tst_test.h"
 
-#ifndef HAVE_STRUCT_SIGNALFD_SIGINFO_SSI_SIGNO
-#define USE_STUB
-#endif
+static int fd_signal = -1;
+static sigset_t mask1;
+static sigset_t mask2;
 
-#ifdef USE_STUB
-int main(void)
+static void check_signal(int fd, uint32_t signal)
 {
-	tst_brkm(TCONF, NULL, "System doesn't support execution of the test");
+	pid_t pid = getpid();
+	ssize_t bytes;
+	struct signalfd_siginfo siginfo;
+
+	SAFE_KILL(pid, signal);
+	bytes = SAFE_READ(0, fd, &siginfo, sizeof(siginfo));
+	TST_EXP_EQ_LI(bytes, sizeof(siginfo));
+	TST_EXP_EQ_LI(siginfo.ssi_signo, signal);
 }
 
-#else
-#if defined USE_OWNIMPL
-#include "lapi/syscalls.h"
-int signalfd(int fd, const sigset_t * mask, int flags)
+static void setup(void)
 {
-	/* Taken from GLIBC. */
-	return tst_syscall(__NR_signalfd, fd, mask, SIGSETSIZE);
+	SAFE_SIGEMPTYSET(&mask1);
+	SAFE_SIGADDSET(&mask1, SIGUSR1);
+	SAFE_SIGPROCMASK(SIG_BLOCK, &mask1, NULL);
+	SAFE_SIGEMPTYSET(&mask2);
+	SAFE_SIGADDSET(&mask2, SIGUSR2);
+	SAFE_SIGPROCMASK(SIG_BLOCK, &mask2, NULL);
 }
-#endif
-
-void cleanup(void);
-void setup(void);
 
-int do_test1(uint32_t sig)
+static void cleanup(void)
 {
-	int sfd_for_next;
-	int sfd;
-	sigset_t mask;
-	pid_t pid;
-	struct signalfd_siginfo fdsi;
-	ssize_t s;
-
-	sigemptyset(&mask);
-	sigaddset(&mask, sig);
-	if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) {
-		tst_brkm(TBROK, cleanup,
-			 "sigprocmask() Failed: errno=%d : %s",
-			 errno, strerror(errno));
-	}
-
-	TEST(signalfd(-1, &mask, 0));
-
-	if ((sfd = TEST_RETURN) == -1) {
-		tst_resm(TFAIL,
-			 "signalfd() Failed, errno=%d : %s",
-			 TEST_ERRNO, strerror(TEST_ERRNO));
-		sfd_for_next = -1;
-		return sfd_for_next;
-
-	} else {
-		tst_resm(TPASS, "signalfd is created successfully");
-		sfd_for_next = sfd;
-		goto out;
-	}
-
-	if (fcntl(sfd, F_SETFL, O_NONBLOCK) == -1) {
-		close(sfd);
-		tst_brkm(TBROK, cleanup,
-			 "setting signalfd nonblocking mode failed: errno=%d : %s",
-			 errno, strerror(errno));
-	}
-
-	pid = getpid();
-	if (kill(pid, sig) == -1) {
-		close(sfd);
-		tst_brkm(TBROK, cleanup,
-			 "kill(self, %s) failed: errno=%d : %s",
-			 strsignal(sig), errno, strerror(errno));
-	}
-
-	s = read(sfd, &fdsi, sizeof(struct signalfd_siginfo));
-	if ((s > 0) && (s != sizeof(struct signalfd_siginfo))) {
-		tst_resm(TFAIL,
-			 "getting incomplete signalfd_siginfo data: "
-			 "actual-size=%zd, expected-size=%zu",
-			 s, sizeof(struct signalfd_siginfo));
-		sfd_for_next = -1;
-		close(sfd);
-		goto out;
-	} else if (s < 0) {
-		if (errno == EAGAIN) {
-			tst_resm(TFAIL,
-				 "signalfd_siginfo data is not delivered yet");
-			sfd_for_next = -1;
-			close(sfd);
-			goto out;
-		} else {
-			close(sfd);
-			tst_brkm(TBROK, cleanup,
-				 "read signalfd_siginfo data failed: errno=%d : %s",
-				 errno, strerror(errno));
-		}
-	} else if (s == 0) {
-		tst_resm(TFAIL, "got EOF unexpectedly");
-		sfd_for_next = -1;
-		close(sfd);
-		goto out;
-	}
-
-	if (fdsi.ssi_signo == sig) {
-		tst_resm(TPASS, "got expected signal");
-		sfd_for_next = sfd;
-		goto out;
-	} else {
-		tst_resm(TFAIL, "got unexpected signal: signal=%d : %s",
-			 fdsi.ssi_signo, strsignal(fdsi.ssi_signo));
-		sfd_for_next = -1;
-		close(sfd);
-		goto out;
-	}
-
-out:
-	return sfd_for_next;
+	if (fd_signal > 0)
+		SAFE_CLOSE(fd_signal);
 }
 
-void do_test2(int fd, uint32_t sig)
+static void verify_signalfd(void)
 {
-	int sfd;
-	sigset_t mask;
-	pid_t pid;
-	struct signalfd_siginfo fdsi;
-	ssize_t s;
-
-	sigemptyset(&mask);
-	sigaddset(&mask, sig);
-	if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) {
-		close(fd);
-		tst_brkm(TBROK, cleanup,
-			 "sigprocmask() Failed: errno=%d : %s",
-			 errno, strerror(errno));
-	}
-
-	TEST(signalfd(fd, &mask, 0));
-
-	if ((sfd = TEST_RETURN) == -1) {
-		tst_resm(TFAIL,
-			 "reassignment the file descriptor by signalfd() failed, errno=%d : %s",
-			 TEST_ERRNO, strerror(TEST_ERRNO));
+	/* create fd */
+	TST_EXP_FD(signalfd(fd_signal, &mask1, 0),
+		"%s", "signalfd() create fd");
+	if (TST_RET == -1)
 		return;
-	} else if (sfd != fd) {
-		tst_resm(TFAIL,
-			 "different fd is returned in reassignment: expected-fd=%d, actual-fd=%d",
-			 fd, sfd);
-		close(sfd);
+	fd_signal = TST_RET;
+	check_signal(fd_signal, SIGUSR1);
+	/* reassign fd */
+	TST_EXP_FD(signalfd(fd_signal, &mask2, 0), "%s",
+		"signalfd() reassign fd");
+	if (TST_RET == -1)
 		return;
-
-	} else {
-		tst_resm(TPASS, "signalfd is successfully reassigned");
-		goto out;
-	}
-
-	pid = getpid();
-	if (kill(pid, sig) == -1) {
-		close(sfd);
-		tst_brkm(TBROK, cleanup,
-			 "kill(self, %s) failed: errno=%d : %s",
-			 strsignal(sig), errno, strerror(errno));
-	}
-
-	s = read(sfd, &fdsi, sizeof(struct signalfd_siginfo));
-	if ((s > 0) && (s != sizeof(struct signalfd_siginfo))) {
-		tst_resm(TFAIL,
-			 "getting incomplete signalfd_siginfo data: "
-			 "actual-size=%zd, expected-size= %zu",
-			 s, sizeof(struct signalfd_siginfo));
-		goto out;
-	} else if (s < 0) {
-		if (errno == EAGAIN) {
-			tst_resm(TFAIL,
-				 "signalfd_siginfo data is not delivered yet");
-			goto out;
-		} else {
-			close(sfd);
-			tst_brkm(TBROK, cleanup,
-				 "read signalfd_siginfo data failed: errno=%d : %s",
-				 errno, strerror(errno));
-		}
-	} else if (s == 0) {
-		tst_resm(TFAIL, "got EOF unexpectedly");
-		goto out;
-	}
-
-	if (fdsi.ssi_signo == sig) {
-		tst_resm(TPASS, "got expected signal");
-		goto out;
-	} else {
-		tst_resm(TFAIL, "got unexpected signal: signal=%d : %s",
-			 fdsi.ssi_signo, strsignal(fdsi.ssi_signo));
-		goto out;
-	}
-
-out:
-	return;
-}
-
-int main(int argc, char **argv)
-{
-	int lc;
-	int sfd;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	setup();
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		sfd = do_test1(SIGUSR1);
-		if (sfd < 0)
-			continue;
-
-		do_test2(sfd, SIGUSR2);
-		close(sfd);
-	}
-
-	cleanup();
-
-	tst_exit();
-}
-
-/*
- * setup() - performs all the ONE TIME setup for this test.
- */
-void setup(void)
-{
-
-	TEST_PAUSE;
-}
-
-/*
- * cleanup() - performs all the ONE TIME cleanup for this test at completion
- * 	       or premature exit.
- */
-void cleanup(void)
-{
-
+	TST_EXP_EQ_LI(TST_RET, fd_signal);
+	check_signal(fd_signal, SIGUSR2);
 }
 
-#endif
+static struct tst_test test = {
+	.test_all = verify_signalfd,
+	.setup = setup,
+	.cleanup = cleanup,
+};
-- 
2.39.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 1/2] signalfd01: Refactor old case with new API

[Cyril Hrubis]

Hi!
> +/*\
> + * [Description]
>   *
> - * HISTORY
> - *	9/2008 Initial version by Masatake YAMATO <yamato@redhat.com>
> + * Verify that signalfd() works as expected.
> + * 1. signalfd() can create fd, and fd can receive signal.
> + * 2. signalfd() can reassign fd, and fd can receive signal.

I've adjusted this comment so that it renders nicely in the html
documentation and pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/2] signalfd: Add negative tests

[Ma Xinjian via ltp]

Add negative cases for signalfd(), when errno is EBADF or EINVAL

Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com>
---
 runtest/syscalls                              |   1 +
 testcases/kernel/syscalls/signalfd/.gitignore |   1 +
 .../kernel/syscalls/signalfd/signalfd02.c     | 115 ++++++++++++++++++
 3 files changed, 117 insertions(+)
 create mode 100644 testcases/kernel/syscalls/signalfd/signalfd02.c

diff --git a/runtest/syscalls b/runtest/syscalls
index a06b046ac..2dac79798 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1487,6 +1487,7 @@ signal05 signal05
 signal06 signal06
 
 signalfd01 signalfd01
+signalfd02 signalfd02
 
 signalfd4_01 signalfd4_01
 signalfd4_02 signalfd4_02
diff --git a/testcases/kernel/syscalls/signalfd/.gitignore b/testcases/kernel/syscalls/signalfd/.gitignore
index 3c9ed737c..959022f41 100644
--- a/testcases/kernel/syscalls/signalfd/.gitignore
+++ b/testcases/kernel/syscalls/signalfd/.gitignore
@@ -1 +1,2 @@
 /signalfd01
+/signalfd02
diff --git a/testcases/kernel/syscalls/signalfd/signalfd02.c b/testcases/kernel/syscalls/signalfd/signalfd02.c
new file mode 100644
index 000000000..9ca943942
--- /dev/null
+++ b/testcases/kernel/syscalls/signalfd/signalfd02.c
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved.
+ * Author: Ma Xinjian <maxj.fnst@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Verify that signalfd(2) fails with
+ *
+ * - EBADF when fd is invalid
+ * - EINVAL when fd is not a valid signalfd file descriptor
+ * - EINVAL when flags are invalid
+ */
+
+#define _GNU_SOURCE
+
+#include <signal.h>
+#include "tst_test.h"
+#include "config.h"
+
+#ifndef HAVE_SIGNALFD
+#define USE_STUB
+#endif
+
+#ifdef HAVE_SYS_SIGNALFD_H
+#include <sys/signalfd.h>
+#elif defined HAVE_LINUX_SIGNALFD_H
+#include <linux/signalfd.h>
+#define USE_OWNIMPL
+#else
+#define USE_STUB
+#endif /* HAVE_SYS_SIGNALFD_H */
+
+#ifndef HAVE_STRUCT_SIGNALFD_SIGINFO_SSI_SIGNO
+#define USE_STUB
+#endif /* HAVE_STRUCT_SIGNALFD_SIGINFO_SSI_SIGNO */
+
+#ifdef USE_STUB
+TST_TEST_TCONF("This system does not support signalfd()");
+#else /* USE_STUB */
+#if defined USE_OWNIMPL
+#include "lapi/syscalls.h"
+int signalfd(int fd, const sigset_t *mask, int flags)
+{
+	return tst_syscall(__NR_signalfd, fd, mask, flags);
+}
+#endif /* USE_OWNIMPL */
+
+#define SIGNAL_FILE "signal_file"
+
+static int fd_ebadf = -2;
+static int fd_einval1;
+static int fd_einval2 = -1;
+
+static sigset_t mask1;
+static sigset_t mask2;
+static sigset_t mask3;
+
+static struct test_case_t {
+	int *fd;
+	sigset_t *mask;
+	int flags;
+	int expected_errno;
+	char *desc;
+} tcases[] = {
+	{&fd_ebadf, &mask1, 0, EBADF, "fd is invalid"},
+	{&fd_einval1, &mask2, 0, EINVAL,
+		"fd is not a valid signalfd file descriptor"},
+	{&fd_einval2, &mask3, -1, EINVAL, "flags are invalid"},
+};
+
+static void setup(void)
+{
+	SAFE_SIGEMPTYSET(&mask1);
+	SAFE_SIGADDSET(&mask1, SIGUSR1);
+	SAFE_SIGPROCMASK(SIG_BLOCK, &mask1, NULL);
+	SAFE_SIGEMPTYSET(&mask2);
+	SAFE_SIGADDSET(&mask2, SIGUSR2);
+	SAFE_SIGPROCMASK(SIG_BLOCK, &mask2, NULL);
+	SAFE_SIGEMPTYSET(&mask2);
+	SAFE_SIGADDSET(&mask3, SIGUSR2);
+	SAFE_SIGPROCMASK(SIG_BLOCK, &mask3, NULL);
+
+	fd_einval1 = SAFE_OPEN(SIGNAL_FILE, O_CREAT, 0777);
+}
+
+static void cleanup(void)
+{
+	if (fd_ebadf > 0)
+		SAFE_CLOSE(fd_ebadf);
+	if (fd_einval1 > 0)
+		SAFE_CLOSE(fd_einval1);
+	if (fd_einval2 > 0)
+		SAFE_CLOSE(fd_einval2);
+}
+
+static void verify_signalfd(unsigned int i)
+{
+	struct test_case_t *tc = &tcases[i];
+
+	TST_EXP_FAIL2(signalfd(*(tc->fd), tc->mask, tc->flags),
+		tc->expected_errno, "%s", tc->desc);
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = verify_signalfd,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_tmpdir = 1,
+};
+
+#endif /* USE_STUB */
-- 
2.39.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] signalfd: Add negative tests

[Andrea Cervesato via ltp]

Hi!

Please take into account the same considerations made in signalfd01 test 
refactoring.

Regards,
Andrea Cervesato

On 5/28/24 12:33, Ma Xinjian via ltp wrote:
> Add negative cases for signalfd(), when errno is EBADF or EINVAL
>
> Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com>
> ---
>   runtest/syscalls                              |   1 +
>   testcases/kernel/syscalls/signalfd/.gitignore |   1 +
>   .../kernel/syscalls/signalfd/signalfd02.c     | 115 ++++++++++++++++++
>   3 files changed, 117 insertions(+)
>   create mode 100644 testcases/kernel/syscalls/signalfd/signalfd02.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index a06b046ac..2dac79798 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1487,6 +1487,7 @@ signal05 signal05
>   signal06 signal06
>   
>   signalfd01 signalfd01
> +signalfd02 signalfd02
>   
>   signalfd4_01 signalfd4_01
>   signalfd4_02 signalfd4_02
> diff --git a/testcases/kernel/syscalls/signalfd/.gitignore b/testcases/kernel/syscalls/signalfd/.gitignore
> index 3c9ed737c..959022f41 100644
> --- a/testcases/kernel/syscalls/signalfd/.gitignore
> +++ b/testcases/kernel/syscalls/signalfd/.gitignore
> @@ -1 +1,2 @@
>   /signalfd01
> +/signalfd02
> diff --git a/testcases/kernel/syscalls/signalfd/signalfd02.c b/testcases/kernel/syscalls/signalfd/signalfd02.c
> new file mode 100644
> index 000000000..9ca943942
> --- /dev/null
> +++ b/testcases/kernel/syscalls/signalfd/signalfd02.c
> @@ -0,0 +1,115 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved.
> + * Author: Ma Xinjian <maxj.fnst@fujitsu.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Verify that signalfd(2) fails with
> + *
> + * - EBADF when fd is invalid
> + * - EINVAL when fd is not a valid signalfd file descriptor
> + * - EINVAL when flags are invalid
> + */
> +
> +#define _GNU_SOURCE
> +
> +#include <signal.h>
> +#include "tst_test.h"
> +#include "config.h"
> +
> +#ifndef HAVE_SIGNALFD
> +#define USE_STUB
> +#endif
> +
> +#ifdef HAVE_SYS_SIGNALFD_H
> +#include <sys/signalfd.h>
> +#elif defined HAVE_LINUX_SIGNALFD_H
> +#include <linux/signalfd.h>
> +#define USE_OWNIMPL
> +#else
> +#define USE_STUB
> +#endif /* HAVE_SYS_SIGNALFD_H */
> +
> +#ifndef HAVE_STRUCT_SIGNALFD_SIGINFO_SSI_SIGNO
> +#define USE_STUB
> +#endif /* HAVE_STRUCT_SIGNALFD_SIGINFO_SSI_SIGNO */
> +
> +#ifdef USE_STUB
> +TST_TEST_TCONF("This system does not support signalfd()");
> +#else /* USE_STUB */
> +#if defined USE_OWNIMPL
> +#include "lapi/syscalls.h"
> +int signalfd(int fd, const sigset_t *mask, int flags)
> +{
> +	return tst_syscall(__NR_signalfd, fd, mask, flags);
> +}
> +#endif /* USE_OWNIMPL */
> +
> +#define SIGNAL_FILE "signal_file"
> +
> +static int fd_ebadf = -2;
> +static int fd_einval1;
> +static int fd_einval2 = -1;
> +
> +static sigset_t mask1;
> +static sigset_t mask2;
> +static sigset_t mask3;
> +
> +static struct test_case_t {
> +	int *fd;
> +	sigset_t *mask;
> +	int flags;
> +	int expected_errno;
> +	char *desc;
> +} tcases[] = {
> +	{&fd_ebadf, &mask1, 0, EBADF, "fd is invalid"},
> +	{&fd_einval1, &mask2, 0, EINVAL,
> +		"fd is not a valid signalfd file descriptor"},
> +	{&fd_einval2, &mask3, -1, EINVAL, "flags are invalid"},
> +};
> +
> +static void setup(void)
> +{
> +	SAFE_SIGEMPTYSET(&mask1);
> +	SAFE_SIGADDSET(&mask1, SIGUSR1);
> +	SAFE_SIGPROCMASK(SIG_BLOCK, &mask1, NULL);
> +	SAFE_SIGEMPTYSET(&mask2);
> +	SAFE_SIGADDSET(&mask2, SIGUSR2);
> +	SAFE_SIGPROCMASK(SIG_BLOCK, &mask2, NULL);
> +	SAFE_SIGEMPTYSET(&mask2);
> +	SAFE_SIGADDSET(&mask3, SIGUSR2);
> +	SAFE_SIGPROCMASK(SIG_BLOCK, &mask3, NULL);
> +
> +	fd_einval1 = SAFE_OPEN(SIGNAL_FILE, O_CREAT, 0777);
> +}
> +
> +static void cleanup(void)
> +{
> +	if (fd_ebadf > 0)
> +		SAFE_CLOSE(fd_ebadf);
> +	if (fd_einval1 > 0)
> +		SAFE_CLOSE(fd_einval1);
> +	if (fd_einval2 > 0)
> +		SAFE_CLOSE(fd_einval2);
> +}
> +
> +static void verify_signalfd(unsigned int i)
> +{
> +	struct test_case_t *tc = &tcases[i];
> +
> +	TST_EXP_FAIL2(signalfd(*(tc->fd), tc->mask, tc->flags),
> +		tc->expected_errno, "%s", tc->desc);
> +}
> +
> +static struct tst_test test = {
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.test = verify_signalfd,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_tmpdir = 1,
> +};
> +
> +#endif /* USE_STUB */



-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 2/2] signalfd: Add negative tests

[Ma Xinjian via ltp]

Add negative cases for signalfd(), when errno is EBADF or EINVAL

Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com>
---
 runtest/syscalls                              |  1 +
 testcases/kernel/syscalls/signalfd/.gitignore |  1 +
 .../kernel/syscalls/signalfd/signalfd02.c     | 82 +++++++++++++++++++
 3 files changed, 84 insertions(+)
 create mode 100644 testcases/kernel/syscalls/signalfd/signalfd02.c

diff --git a/runtest/syscalls b/runtest/syscalls
index a06b046ac..2dac79798 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1487,6 +1487,7 @@ signal05 signal05
 signal06 signal06
 
 signalfd01 signalfd01
+signalfd02 signalfd02
 
 signalfd4_01 signalfd4_01
 signalfd4_02 signalfd4_02
diff --git a/testcases/kernel/syscalls/signalfd/.gitignore b/testcases/kernel/syscalls/signalfd/.gitignore
index 3c9ed737c..959022f41 100644
--- a/testcases/kernel/syscalls/signalfd/.gitignore
+++ b/testcases/kernel/syscalls/signalfd/.gitignore
@@ -1 +1,2 @@
 /signalfd01
+/signalfd02
diff --git a/testcases/kernel/syscalls/signalfd/signalfd02.c b/testcases/kernel/syscalls/signalfd/signalfd02.c
new file mode 100644
index 000000000..527e1aff9
--- /dev/null
+++ b/testcases/kernel/syscalls/signalfd/signalfd02.c
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved.
+ * Author: Ma Xinjian <maxj.fnst@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Verify that signalfd(2) fails with
+ *
+ * - EBADF when fd is invalid
+ * - EINVAL when fd is not a valid signalfd file descriptor
+ * - EINVAL when flags are invalid
+ */
+
+#include <sys/signalfd.h>
+#include "tst_test.h"
+
+#define SIGNAL_FILE "signal_file"
+
+static int fd_ebadf = -2;
+static int fd_einval1;
+static int fd_einval2 = -1;
+
+static sigset_t mask1;
+static sigset_t mask2;
+static sigset_t mask3;
+
+static struct test_case_t {
+	int *fd;
+	sigset_t *mask;
+	int flags;
+	int expected_errno;
+	char *desc;
+} tcases[] = {
+	{&fd_ebadf, &mask1, 0, EBADF, "fd is invalid"},
+	{&fd_einval1, &mask2, 0, EINVAL,
+		"fd is not a valid signalfd file descriptor"},
+	{&fd_einval2, &mask3, -1, EINVAL, "flags are invalid"},
+};
+
+static void setup(void)
+{
+	SAFE_SIGEMPTYSET(&mask1);
+	SAFE_SIGADDSET(&mask1, SIGUSR1);
+	SAFE_SIGPROCMASK(SIG_BLOCK, &mask1, NULL);
+	SAFE_SIGEMPTYSET(&mask2);
+	SAFE_SIGADDSET(&mask2, SIGUSR2);
+	SAFE_SIGPROCMASK(SIG_BLOCK, &mask2, NULL);
+	SAFE_SIGEMPTYSET(&mask2);
+	SAFE_SIGADDSET(&mask3, SIGUSR2);
+	SAFE_SIGPROCMASK(SIG_BLOCK, &mask3, NULL);
+
+	fd_einval1 = SAFE_OPEN(SIGNAL_FILE, O_CREAT, 0777);
+}
+
+static void cleanup(void)
+{
+	if (fd_ebadf > 0)
+		SAFE_CLOSE(fd_ebadf);
+	if (fd_einval1 > 0)
+		SAFE_CLOSE(fd_einval1);
+	if (fd_einval2 > 0)
+		SAFE_CLOSE(fd_einval2);
+}
+
+static void verify_signalfd(unsigned int i)
+{
+	struct test_case_t *tc = &tcases[i];
+
+	TST_EXP_FAIL2(signalfd(*(tc->fd), tc->mask, tc->flags),
+		tc->expected_errno, "%s", tc->desc);
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = verify_signalfd,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_tmpdir = 1,
+};
-- 
2.39.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 2/2] signalfd: Add negative tests

[Cyril Hrubis]

Hi!
Pushed with changes described below, thanks.

- there is no need for three masks we just need one valid mask
  + moved the mask structure into a guarded buffer

- it's pointless to attempt to close the fd_einval2 and fd_ebadf
  as these variables are never assigned a valid fd

diff --git a/testcases/kernel/syscalls/signalfd/signalfd02.c b/testcases/kernel/syscalls/signalfd/signalfd02.c
index 527e1aff9..fdd15781c 100644
--- a/testcases/kernel/syscalls/signalfd/signalfd02.c
+++ b/testcases/kernel/syscalls/signalfd/signalfd02.c
@@ -7,7 +7,7 @@
 /*\
  * [Description]
  *
- * Verify that signalfd(2) fails with
+ * Verify that signalfd(2) fails with:
  *
  * - EBADF when fd is invalid
  * - EINVAL when fd is not a valid signalfd file descriptor
@@ -23,53 +23,40 @@ static int fd_ebadf = -2;
 static int fd_einval1;
 static int fd_einval2 = -1;
 
-static sigset_t mask1;
-static sigset_t mask2;
-static sigset_t mask3;
+static sigset_t *mask;
 
 static struct test_case_t {
        int *fd;
-       sigset_t *mask;
        int flags;
        int expected_errno;
        char *desc;
 } tcases[] = {
-       {&fd_ebadf, &mask1, 0, EBADF, "fd is invalid"},
-       {&fd_einval1, &mask2, 0, EINVAL,
+       {&fd_ebadf, 0, EBADF, "fd is invalid"},
+       {&fd_einval1, 0, EINVAL,
                "fd is not a valid signalfd file descriptor"},
-       {&fd_einval2, &mask3, -1, EINVAL, "flags are invalid"},
+       {&fd_einval2, -1, EINVAL, "flags are invalid"},
 };
 
 static void setup(void)
 {
-       SAFE_SIGEMPTYSET(&mask1);
-       SAFE_SIGADDSET(&mask1, SIGUSR1);
-       SAFE_SIGPROCMASK(SIG_BLOCK, &mask1, NULL);
-       SAFE_SIGEMPTYSET(&mask2);
-       SAFE_SIGADDSET(&mask2, SIGUSR2);
-       SAFE_SIGPROCMASK(SIG_BLOCK, &mask2, NULL);
-       SAFE_SIGEMPTYSET(&mask2);
-       SAFE_SIGADDSET(&mask3, SIGUSR2);
-       SAFE_SIGPROCMASK(SIG_BLOCK, &mask3, NULL);
+       SAFE_SIGEMPTYSET(mask);
+       SAFE_SIGADDSET(mask, SIGUSR1);
+       SAFE_SIGPROCMASK(SIG_BLOCK, mask, NULL);
 
        fd_einval1 = SAFE_OPEN(SIGNAL_FILE, O_CREAT, 0777);
 }
 
 static void cleanup(void)
 {
-       if (fd_ebadf > 0)
-               SAFE_CLOSE(fd_ebadf);
        if (fd_einval1 > 0)
                SAFE_CLOSE(fd_einval1);
-       if (fd_einval2 > 0)
-               SAFE_CLOSE(fd_einval2);
 }
 
 static void verify_signalfd(unsigned int i)
 {
        struct test_case_t *tc = &tcases[i];
 
-       TST_EXP_FAIL2(signalfd(*(tc->fd), tc->mask, tc->flags),
+       TST_EXP_FAIL2(signalfd(*(tc->fd), mask, tc->flags),
                tc->expected_errno, "%s", tc->desc);
 }
 
@@ -79,4 +66,8 @@ static struct tst_test test = {
        .setup = setup,
        .cleanup = cleanup,
        .needs_tmpdir = 1,
+       .bufs = (struct tst_buffers []) {
+               {&mask, .size = sizeof(sigset_t)},
+               {}
+       }
 };


-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 0/2] signalfd: Improve testsuite for signalfd

[Cyril Hrubis]

Hi!
> Currently, testsuite for signalfd is written in old API, and only
> contains case for normal situation.
> 
> So I refactor the existed case with new API, and add case for negative
> situations.

Good work, but we stil miss important testcases for the signalfd namely:

- blocking read() on signalfd waits until signal arrives
- signalfd works well with select()/poll()

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp