[v1] rt_sigpending02: reuse code from sigpending02

Rather than forking the code of sigpending02 completely, reuse its code
for rt_sigpending02 by conditionally compiling the syscalls accordingly.

That way we ensure tests written for either rt_sigpending or sigpending
are also considered for the respective other.

Signed-off-by: Matthias Maennich <maennich@google.com>
---
 .../kernel/syscalls/rt_sigpending/Makefile    |  8 ++++
 .../syscalls/rt_sigpending/rt_sigpending02.c  | 48 -------------------
 testcases/kernel/syscalls/sigpending/Makefile |  2 +
 .../kernel/syscalls/sigpending/sigpending02.c | 16 +++++--
 4 files changed, 23 insertions(+), 51 deletions(-)
 delete mode 100644 testcases/kernel/syscalls/rt_sigpending/rt_sigpending02.c

Hi Matthias,

On 03/08/2019 08:38 AM, 'Matthias Maennich' via kernel-team wrote:
> Rather than forking the code of sigpending02 completely, reuse its code
> for rt_sigpending02 by conditionally compiling the syscalls accordingly.
> 
> That way we ensure tests written for either rt_sigpending or sigpending
> are also considered for the respective other.
> 
> Signed-off-by: Matthias Maennich <maennich@google.com>

Cyril recently proposed a test multiplex function which hasn't yet gone in:

http://lists.linux.it/pipermail/ltp/2019-March/011119.html

This would remove the need for the custom Makefile bit.

One downside with this multiplexed approach is that we then don't have 
an entry in the testcases/kernel/syscalls/ directory for all syscalls 
which can cause some confusion, but that could perhaps be addressed by 
adding symlinks for the missing ones.

> ---
>   .../kernel/syscalls/rt_sigpending/Makefile    |  8 ++++
>   .../syscalls/rt_sigpending/rt_sigpending02.c  | 48 -------------------
>   testcases/kernel/syscalls/sigpending/Makefile |  2 +
>   .../kernel/syscalls/sigpending/sigpending02.c | 16 +++++--
>   4 files changed, 23 insertions(+), 51 deletions(-)
>   delete mode 100644 testcases/kernel/syscalls/rt_sigpending/rt_sigpending02.c
> 
> diff --git a/testcases/kernel/syscalls/rt_sigpending/Makefile b/testcases/kernel/syscalls/rt_sigpending/Makefile
> index 60c2e0140d85..57ba3dac2e41 100644
> --- a/testcases/kernel/syscalls/rt_sigpending/Makefile
> +++ b/testcases/kernel/syscalls/rt_sigpending/Makefile
> @@ -4,4 +4,12 @@
>   top_srcdir		?= ../../../..
>   
>   include $(top_srcdir)/include/mk/testcases.mk
> +
> +CPPFLAGS += -DTEST_RT_SIGPENDING
> +
> +rt_sigpending02: $(abs_srcdir)/../sigpending/sigpending02.c
> +	$(LINK.c) $^ $(LOADLIBES) $(LDLIBS) $(OUTPUT_OPTION)
> +
> +MAKE_TARGETS := rt_sigpending02
> +
>   include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/syscalls/rt_sigpending/rt_sigpending02.c b/testcases/kernel/syscalls/rt_sigpending/rt_sigpending02.c
> deleted file mode 100644
> index 06d1c1578f4d..000000000000
> --- a/testcases/kernel/syscalls/rt_sigpending/rt_sigpending02.c
> +++ /dev/null
> @@ -1,48 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - *  Copyright (c) International Business Machines  Corp., 2002
> - *
> - * AUTHORS
> - *	Paul Larson
> - *
> - * DESCRIPTION
> - *	Test to see that the proper errors are returned by rt_sigpending
> - *
> - *	Test 1:
> - *		Call rt_sigpending(sigset_t*=-1, SIGSETSIZE),
> - *		it should return -1 with errno EFAULT
> - */
> -
> -#include <errno.h>
> -#include <signal.h>
> -#include <sys/types.h>
> -
> -#include "tst_test.h"
> -#include "ltp_signal.h"
> -#include "lapi/syscalls.h"
> -
> -static void run(void)
> -{
> -	/* set sigset to point to an invalid location */
> -	sigset_t *sigset = (sigset_t *) - 1;
> -
> -	TEST(tst_syscall(__NR_rt_sigpending, sigset, SIGSETSIZE));
> -
> -	/* check return code */
> -	if (TST_RET == -1) {
> -		if (TST_ERR != EFAULT) {
> -			tst_res(TFAIL | TTERRNO,
> -				"rt_sigpending() Failed with wrong errno, "
> -				"expected errno=%d, got ", EFAULT);
> -		} else {
> -			tst_res(TPASS | TTERRNO, "expected failure");
> -		}
> -	} else {
> -		tst_res(TFAIL,
> -			"rt_sigpending() Failed, expected return value=-1, got %ld", TST_RET);
> -	}
> -}
> -
> -static struct tst_test test = {
> -	.test_all = run
> -};
> diff --git a/testcases/kernel/syscalls/sigpending/Makefile b/testcases/kernel/syscalls/sigpending/Makefile
> index bd617d806675..00a7d5e2b538 100644
> --- a/testcases/kernel/syscalls/sigpending/Makefile
> +++ b/testcases/kernel/syscalls/sigpending/Makefile
> @@ -20,4 +20,6 @@ top_srcdir		?= ../../../..
>   
>   include $(top_srcdir)/include/mk/testcases.mk
>   
> +CPPFLAGS += -DTEST_SIGPENDING
> +
>   include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/syscalls/sigpending/sigpending02.c b/testcases/kernel/syscalls/sigpending/sigpending02.c
> index e7ec0d6819bd..cc50870b107a 100644
> --- a/testcases/kernel/syscalls/sigpending/sigpending02.c
> +++ b/testcases/kernel/syscalls/sigpending/sigpending02.c
> @@ -6,7 +6,10 @@
>    *	Paul Larson
>    *
>    * DESCRIPTION
> - *	Test to see that the proper errors are returned by sigpending
> + *	Test to see that the proper errors are returned by sigpending. All the
> + *	tests can also be compiled to use the rt_sigpending syscall instead. To
> + *	simplify the documentation, only sigpending() is usually mentioned
> + *	below.
>    *
>    *	Test 1:
>    *		Call sigpending(sigset_t*=-1), it should return -1 with errno EFAULT
> @@ -17,6 +20,7 @@
>   #include <sys/types.h>
>   
>   #include "tst_test.h"
> +#include "ltp_signal.h"
>   #include "lapi/syscalls.h"
>   
>   static void run(void)
> @@ -24,20 +28,26 @@ static void run(void)
>   	/* set sigset to point to an invalid location */
>   	sigset_t *sigset = (sigset_t *) - 1;
>   
> +#if defined(TEST_SIGPENDING)
>   	TEST(tst_syscall(__NR_sigpending, sigset));
> +#elif defined(TEST_RT_SIGPENDING)
> +	TEST(tst_syscall(__NR_rt_sigpending, sigset, SIGSETSIZE));
> +#else
> +#error Neither TEST_SIGPENDING nor TEST_RT_SIGPENDING is defined!
> +#endif
>   
>   	/* check return code */
>   	if (TST_RET == -1) {
>   		if (TST_ERR != EFAULT) {
>   			tst_res(TFAIL | TTERRNO,
> -				"sigpending() Failed with wrong errno, "
> +				"syscall failed with wrong errno, "
>   				"expected errno=%d, got ", EFAULT);
>   		} else {
>   			tst_res(TPASS | TTERRNO, "expected failure");
>   		}
>   	} else {
>   		tst_res(TFAIL,
> -			"sigpending() Failed, expected return value=-1, got %ld", TST_RET);
> +			"syscall failed, expected return value=-1, got %ld", TST_RET);
>   	}
>   }
>   
>

Hi!
> One downside with this multiplexed approach is that we then don't have 
> an entry in the testcases/kernel/syscalls/ directory for all syscalls 
> which can cause some confusion, but that could perhaps be addressed by 
> adding symlinks for the missing ones.

Actually my long term plan is to include metadata in the testcases which
would, among other things, describe which syscalls/libcalls the tests is
excercising and I want this information to be propagated to the test
runner as well, so instead of relying on one binary file per syscall we
would have proper metadata describing the tests.

And the biggest problem here is that it looks that there is very little
interest in investing time into this approach. I've send a (quick and
dirty) RFC patch that tried to show a direction for such work, but
nearly nobody replied to it, so I postponed the work a bit.

See:

http://patchwork.ozlabs.org/project/ltp/list/?series=78493

Hi!

On Wed, Mar 13, 2019 at 10:42:28AM +0100, Cyril Hrubis wrote:
> Hi!
> > One downside with this multiplexed approach is that we then don't have 
> > an entry in the testcases/kernel/syscalls/ directory for all syscalls 
> > which can cause some confusion, but that could perhaps be addressed by 
> > adding symlinks for the missing ones.
I am not sure multiplexing is the right approach here (not saying it is not!).
In case of (rt_)sigpending, I would like to see them as separate binaries that
effectively do disjunct things. There might be the case that sigpending is not
available on that particular kernel and rt_sigpending is. I would like the
sigpending to fail with TCONF and the rt_sigpending to TPASS in that case. Is
that something that can be achieved with multiplexing?

I was already working on a v2 of this patch set to add a further test case and
will send this out shortly. I would like to reconsider multiplexing at a later
time and for now follow the pattern of other syscall related tests like
sigwait, sigtimedwait, rt_sigtimedwait.

> 
> Actually my long term plan is to include metadata in the testcases which
> would, among other things, describe which syscalls/libcalls the tests is
> excercising and I want this information to be propagated to the test
> runner as well, so instead of relying on one binary file per syscall we
> would have proper metadata describing the tests.
> 
> And the biggest problem here is that it looks that there is very little
> interest in investing time into this approach. I've send a (quick and
> dirty) RFC patch that tried to show a direction for such work, but
> nearly nobody replied to it, so I postponed the work a bit.
> 
> See:
> 
> http://patchwork.ozlabs.org/project/ltp/list/?series=78493
That looks actually promising. I will have a more detailed look these days!

On 03/13/2019 04:42 AM, Matthias Maennich wrote:
> I am not sure multiplexing is the right approach here (not saying it is not!).
> In case of (rt_)sigpending, I would like to see them as separate binaries that
> effectively do disjunct things.

Ok I don't have a strong feeling either way, keeping them as separate 
binaries seems a little more complex as far as the build goes (i.e. 
Makefile stuff that has to reach into a sibling directory), but it's 
also slightly clearer there are tests for both syscalls when perusing 
the source.

> There might be the case that sigpending is not available on that
> particular kernel and rt_sigpending is. I would like the sigpending
> to fail with TCONF and the rt_sigpending to TPASS in that  case. Is
> that something that can be achieved with multiplexing?

Yep tests can return multiple results (have multiple sub-tests) so this 
is doable.

> I was already working on a v2 of this patch set to add a further test case and
> will send this out shortly. I would like to reconsider multiplexing at a later
> time and for now follow the pattern of other syscall related tests like
> sigwait, sigtimedwait, rt_sigtimedwait.

SGTM

cheers,
steve