Message ID | 20240821061523.2650568-2-dev.jain@arm.com |
---|---|
State | New |
Headers | show |
Series | Add test to distinguish between thread's signal mask and ucontext_t | expand |
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
On 8/22/24 08:33, Shuah Khan wrote: > 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. Okay. > > 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. After looking into some git history, now I understand that "sas" actually has some meaning, although I still can't find its full-form :) I thought that sigaltstack would be a better name, but I guess sas is a subset of sigaltstack as part of SA_ONSTACK. So, let us drop the renaming of the test. >> >> 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 >
On 8/22/24 05:10, Dev Jain wrote: > > On 8/22/24 08:33, Shuah Khan wrote: >> 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. > > Okay. > > >> >> 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. > > After looking into some git history, now I understand that "sas" actually > has some meaning, although I still can't find its full-form :) I thought that > sigaltstack would be a better name, but I guess sas is a subset of sigaltstack > as part of SA_ONSTACK. So, let us drop the renaming of the test. > I assume you will be sending a new v6 patch series without the renaming and just the new test? thanks, -- Shuah
On 8/27/24 17:08, Shuah Khan wrote: > On 8/22/24 05:10, Dev Jain wrote: >> >> On 8/22/24 08:33, Shuah Khan wrote: >>> 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. >> >> Okay. >> >> >>> >>> 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. >> >> After looking into some git history, now I understand that "sas" >> actually >> has some meaning, although I still can't find its full-form :) I >> thought that >> sigaltstack would be a better name, but I guess sas is a subset of >> sigaltstack >> as part of SA_ONSTACK. So, let us drop the renaming of the test. >> > > I assume you will be sending a new v6 patch series without the > renaming and just the > new test? I had already sent it: https://lore.kernel.org/all/20240822121415.3589190-1-dev.jain@arm.com/ > > thanks, > -- Shuah >
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