Linux Kernel Selftest development
 help / color / mirror / Atom feed
From: Shuah Khan <skhan@linuxfoundation.org>
To: Dev Jain <dev.jain@arm.com>, shuah@kernel.org, oleg@redhat.com
Cc: mingo@kernel.org, tglx@linutronix.de, mark.rutland@arm.com,
	ryan.roberts@arm.com, broonie@kernel.org, suzuki.poulose@arm.com,
	Anshuman.Khandual@arm.com, DeepakKumar.Mishra@arm.com,
	aneesh.kumar@kernel.org, linux-kselftest@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH v5 1/2] selftests: Rename sigaltstack to generic signal
Date: Wed, 21 Aug 2024 21:03:51 -0600	[thread overview]
Message-ID: <1ac911c2-9d9c-4408-8697-1e90b3ae3e8d@linuxfoundation.org> (raw)
In-Reply-To: <20240821061523.2650568-2-dev.jain@arm.com>

On 8/21/24 00:15, Dev Jain wrote:
> Rename sigaltstack to signal, and rename the existing test to
> sigaltstack.c.

Can you elaborate on the benefits if renaming the test?

Also you have such a good information in the cover-letter for this
patch - it would be good to include it in the change log for this
one or the new test.

The new test itself is good. I don't understand the value of renaming.
I can see the problems due to not being able to fix stables if the
existing test needs fixing. If there are good reasons for renaming,
I am all for it.
> 
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> Reviewed-by: Mark Brown <broonie@kernel.org>
> ---
>   tools/testing/selftests/Makefile                                | 2 +-
>   tools/testing/selftests/{sigaltstack => signal}/.gitignore      | 2 +-
>   tools/testing/selftests/{sigaltstack => signal}/Makefile        | 2 +-
>   .../selftests/{sigaltstack => signal}/current_stack_pointer.h   | 0
>   .../selftests/{sigaltstack/sas.c => signal/sigaltstack.c}       | 0
>   5 files changed, 3 insertions(+), 3 deletions(-)
>   rename tools/testing/selftests/{sigaltstack => signal}/.gitignore (76%)
>   rename tools/testing/selftests/{sigaltstack => signal}/Makefile (72%)
>   rename tools/testing/selftests/{sigaltstack => signal}/current_stack_pointer.h (100%)
>   rename tools/testing/selftests/{sigaltstack/sas.c => signal/sigaltstack.c} (100%)
> 
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index bc8fe9e8f7f2..edbe30fb3304 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -87,7 +87,7 @@ TARGETS += rtc
>   TARGETS += rust
>   TARGETS += seccomp
>   TARGETS += sgx
> -TARGETS += sigaltstack
> +TARGETS += signal
>   TARGETS += size
>   TARGETS += sparc64
>   TARGETS += splice
> diff --git a/tools/testing/selftests/sigaltstack/.gitignore b/tools/testing/selftests/signal/.gitignore
> similarity index 76%
> rename from tools/testing/selftests/sigaltstack/.gitignore
> rename to tools/testing/selftests/signal/.gitignore
> index 50a19a8888ce..98a7bbc4f325 100644
> --- a/tools/testing/selftests/sigaltstack/.gitignore
> +++ b/tools/testing/selftests/signal/.gitignore
> @@ -1,2 +1,2 @@
>   # SPDX-License-Identifier: GPL-2.0-only
> -sas
> +sigaltstack
> diff --git a/tools/testing/selftests/sigaltstack/Makefile b/tools/testing/selftests/signal/Makefile
> similarity index 72%
> rename from tools/testing/selftests/sigaltstack/Makefile
> rename to tools/testing/selftests/signal/Makefile
> index 3e96d5d47036..dd6be992fd81 100644
> --- a/tools/testing/selftests/sigaltstack/Makefile
> +++ b/tools/testing/selftests/signal/Makefile
> @@ -1,6 +1,6 @@
>   # SPDX-License-Identifier: GPL-2.0-only
>   CFLAGS = -Wall
> -TEST_GEN_PROGS = sas
> +TEST_GEN_PROGS = sigaltstack
>   
>   include ../lib.mk
>   
> diff --git a/tools/testing/selftests/sigaltstack/current_stack_pointer.h b/tools/testing/selftests/signal/current_stack_pointer.h
> similarity index 100%
> rename from tools/testing/selftests/sigaltstack/current_stack_pointer.h
> rename to tools/testing/selftests/signal/current_stack_pointer.h
> diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/signal/sigaltstack.c
> similarity index 100%
> rename from tools/testing/selftests/sigaltstack/sas.c
> rename to tools/testing/selftests/signal/sigaltstack.c

thanks,
-- Shuah


  reply	other threads:[~2024-08-22  3:03 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-21  6:15 [PATCH v5 0/2] Add test to distinguish between thread's signal mask and ucontext_t Dev Jain
2024-08-21  6:15 ` [PATCH v5 1/2] selftests: Rename sigaltstack to generic signal Dev Jain
2024-08-22  3:03   ` Shuah Khan [this message]
2024-08-22 11:10     ` Dev Jain
2024-08-27 11:38       ` Shuah Khan
2024-08-27 11:43         ` Dev Jain
2024-08-21  6:15 ` [PATCH v5 2/2] selftests: Add a test mangling with uc_sigmask Dev Jain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1ac911c2-9d9c-4408-8697-1e90b3ae3e8d@linuxfoundation.org \
    --to=skhan@linuxfoundation.org \
    --cc=Anshuman.Khandual@arm.com \
    --cc=DeepakKumar.Mishra@arm.com \
    --cc=aneesh.kumar@kernel.org \
    --cc=broonie@kernel.org \
    --cc=dev.jain@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=ryan.roberts@arm.com \
    --cc=shuah@kernel.org \
    --cc=suzuki.poulose@arm.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox