| Message ID | 20240909-regen_shutup_lsp-v1-1-d65ad07b822f@suse.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Remove cleanup.c inclusion from syscalls generation | expand |
Hi! > Remove cleanup.c from syscalls.h as it is part of the old library and > its usage is discouraged. LSP(s) supporting C language correctly flag > this file as problematic. This patch ensures that the lapi/syscalls/regen.sh > script will no longer include cleanup.c, preventing unnecessary usage in > generated headers. > > Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> > --- > include/lapi/syscalls/regen.sh | 6 ++++-- Can we remove the cleanup.c as well? It does not seem to be used anywhere after this change. > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/include/lapi/syscalls/regen.sh b/include/lapi/syscalls/regen.sh > index 97027e2f3..0dd3269bb 100755 > --- a/include/lapi/syscalls/regen.sh > +++ b/include/lapi/syscalls/regen.sh > @@ -33,7 +33,6 @@ cat << EOF > "${output_pid}" > #include <errno.h> > #include <sys/syscall.h> > #include <asm/unistd.h> > -#include "cleanup.c" > > #ifdef TST_TEST_H__ > #define TST_SYSCALL_BRK__(NR, SNR) ({ \\ > @@ -41,8 +40,11 @@ cat << EOF > "${output_pid}" > "syscall(%d) " SNR " not supported on your arch", NR); \\ > }) > #else > +static void dummy_cleanup(void) __attribute__ ((unused)); I do not think that we need this part, the function is always used because it's passed to the tst_brkm(). > +static void dummy_cleanup(void) {} > + > #define TST_SYSCALL_BRK__(NR, SNR) ({ \\ > - tst_brkm(TCONF, CLEANUP, \\ > + tst_brkm(TCONF, dummy_cleanup, \\ > "syscall(%d) " SNR " not supported on your arch", NR); \\ > }) > #endif > > --- > base-commit: d3f1f93eda69905932bde4f66b44d72f9211909a > change-id: 20240909-regen_shutup_lsp-a35606a887b6 > > Best regards, > -- > Andrea Cervesato <andrea.cervesato@suse.com> > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
Hi! On 9/9/24 16:14, Cyril Hrubis wrote: > Hi! >> Remove cleanup.c from syscalls.h as it is part of the old library and >> its usage is discouraged. LSP(s) supporting C language correctly flag >> this file as problematic. This patch ensures that the lapi/syscalls/regen.sh >> script will no longer include cleanup.c, preventing unnecessary usage in >> generated headers. >> >> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> >> --- >> include/lapi/syscalls/regen.sh | 6 ++++-- > Can we remove the cleanup.c as well? It does not seem to be used > anywhere after this change. Do we want to touch old library? I actually don't mind since we are going to remove everything there anyway, one day. >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/include/lapi/syscalls/regen.sh b/include/lapi/syscalls/regen.sh >> index 97027e2f3..0dd3269bb 100755 >> --- a/include/lapi/syscalls/regen.sh >> +++ b/include/lapi/syscalls/regen.sh >> @@ -33,7 +33,6 @@ cat << EOF > "${output_pid}" >> #include <errno.h> >> #include <sys/syscall.h> >> #include <asm/unistd.h> >> -#include "cleanup.c" >> >> #ifdef TST_TEST_H__ >> #define TST_SYSCALL_BRK__(NR, SNR) ({ \\ >> @@ -41,8 +40,11 @@ cat << EOF > "${output_pid}" >> "syscall(%d) " SNR " not supported on your arch", NR); \\ >> }) >> #else >> +static void dummy_cleanup(void) __attribute__ ((unused)); > I do not think that we need this part, the function is always used > because it's passed to the tst_brkm(). I actually had some warnings and that is needed unfortunately. >> +static void dummy_cleanup(void) {} >> + >> #define TST_SYSCALL_BRK__(NR, SNR) ({ \\ >> - tst_brkm(TCONF, CLEANUP, \\ >> + tst_brkm(TCONF, dummy_cleanup, \\ >> "syscall(%d) " SNR " not supported on your arch", NR); \\ >> }) >> #endif >> >> --- >> base-commit: d3f1f93eda69905932bde4f66b44d72f9211909a >> change-id: 20240909-regen_shutup_lsp-a35606a887b6 >> >> Best regards, >> -- >> Andrea Cervesato <andrea.cervesato@suse.com> >> >> >> -- >> Mailing list info: https://lists.linux.it/listinfo/ltp Andrea
Hi! > > Can we remove the cleanup.c as well? It does not seem to be used > > anywhere after this change. > Do we want to touch old library? I actually don't mind since we are > going to remove everything there anyway, one day. I would remove any parts that are not used anymore. That way it's more clear how much old API mess we have to clean up. > >> 1 file changed, 4 insertions(+), 2 deletions(-) > >> > >> diff --git a/include/lapi/syscalls/regen.sh b/include/lapi/syscalls/regen.sh > >> index 97027e2f3..0dd3269bb 100755 > >> --- a/include/lapi/syscalls/regen.sh > >> +++ b/include/lapi/syscalls/regen.sh > >> @@ -33,7 +33,6 @@ cat << EOF > "${output_pid}" > >> #include <errno.h> > >> #include <sys/syscall.h> > >> #include <asm/unistd.h> > >> -#include "cleanup.c" > >> > >> #ifdef TST_TEST_H__ > >> #define TST_SYSCALL_BRK__(NR, SNR) ({ \\ > >> @@ -41,8 +40,11 @@ cat << EOF > "${output_pid}" > >> "syscall(%d) " SNR " not supported on your arch", NR); \\ > >> }) > >> #else > >> +static void dummy_cleanup(void) __attribute__ ((unused)); > > I do not think that we need this part, the function is always used > > because it's passed to the tst_brkm(). > I actually had some warnings and that is needed unfortunately. Hmm, that is strange, the function is passed to the tst_brkm(). Maybe the compiler is smart enough to figure out that it's not used when the header is included and the tst_syscall() is not used. The standard way how to define functions in header is 'static inline' so maybe drop this line and add inline to the function definition instead? > >> +static void dummy_cleanup(void) {} > >> + > >> #define TST_SYSCALL_BRK__(NR, SNR) ({ \\ > >> - tst_brkm(TCONF, CLEANUP, \\ > >> + tst_brkm(TCONF, dummy_cleanup, \\ > >> "syscall(%d) " SNR " not supported on your arch", NR); \\ > >> }) > >> #endif > >> > >> --- > >> base-commit: d3f1f93eda69905932bde4f66b44d72f9211909a > >> change-id: 20240909-regen_shutup_lsp-a35606a887b6 > >> > >> Best regards, > >> -- > >> Andrea Cervesato <andrea.cervesato@suse.com> > >> > >> > >> -- > >> Mailing list info: https://lists.linux.it/listinfo/ltp > Andrea
Ok, thanks. v2 sent. Andrea On 9/10/24 10:41, Cyril Hrubis wrote: > Hi! >>> Can we remove the cleanup.c as well? It does not seem to be used >>> anywhere after this change. >> Do we want to touch old library? I actually don't mind since we are >> going to remove everything there anyway, one day. > I would remove any parts that are not used anymore. That way it's more > clear how much old API mess we have to clean up. > >>>> 1 file changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/include/lapi/syscalls/regen.sh b/include/lapi/syscalls/regen.sh >>>> index 97027e2f3..0dd3269bb 100755 >>>> --- a/include/lapi/syscalls/regen.sh >>>> +++ b/include/lapi/syscalls/regen.sh >>>> @@ -33,7 +33,6 @@ cat << EOF > "${output_pid}" >>>> #include <errno.h> >>>> #include <sys/syscall.h> >>>> #include <asm/unistd.h> >>>> -#include "cleanup.c" >>>> >>>> #ifdef TST_TEST_H__ >>>> #define TST_SYSCALL_BRK__(NR, SNR) ({ \\ >>>> @@ -41,8 +40,11 @@ cat << EOF > "${output_pid}" >>>> "syscall(%d) " SNR " not supported on your arch", NR); \\ >>>> }) >>>> #else >>>> +static void dummy_cleanup(void) __attribute__ ((unused)); >>> I do not think that we need this part, the function is always used >>> because it's passed to the tst_brkm(). >> I actually had some warnings and that is needed unfortunately. > Hmm, that is strange, the function is passed to the tst_brkm(). Maybe > the compiler is smart enough to figure out that it's not used when the > header is included and the tst_syscall() is not used. > > The standard way how to define functions in header is 'static inline' so > maybe drop this line and add inline to the function definition instead? > >>>> +static void dummy_cleanup(void) {} >>>> + >>>> #define TST_SYSCALL_BRK__(NR, SNR) ({ \\ >>>> - tst_brkm(TCONF, CLEANUP, \\ >>>> + tst_brkm(TCONF, dummy_cleanup, \\ >>>> "syscall(%d) " SNR " not supported on your arch", NR); \\ >>>> }) >>>> #endif >>>> >>>> --- >>>> base-commit: d3f1f93eda69905932bde4f66b44d72f9211909a >>>> change-id: 20240909-regen_shutup_lsp-a35606a887b6 >>>> >>>> Best regards, >>>> -- >>>> Andrea Cervesato <andrea.cervesato@suse.com> >>>> >>>> >>>> -- >>>> Mailing list info: https://lists.linux.it/listinfo/ltp >> Andrea
diff --git a/include/lapi/syscalls/regen.sh b/include/lapi/syscalls/regen.sh index 97027e2f3..0dd3269bb 100755 --- a/include/lapi/syscalls/regen.sh +++ b/include/lapi/syscalls/regen.sh @@ -33,7 +33,6 @@ cat << EOF > "${output_pid}" #include <errno.h> #include <sys/syscall.h> #include <asm/unistd.h> -#include "cleanup.c" #ifdef TST_TEST_H__ #define TST_SYSCALL_BRK__(NR, SNR) ({ \\ @@ -41,8 +40,11 @@ cat << EOF > "${output_pid}" "syscall(%d) " SNR " not supported on your arch", NR); \\ }) #else +static void dummy_cleanup(void) __attribute__ ((unused)); +static void dummy_cleanup(void) {} + #define TST_SYSCALL_BRK__(NR, SNR) ({ \\ - tst_brkm(TCONF, CLEANUP, \\ + tst_brkm(TCONF, dummy_cleanup, \\ "syscall(%d) " SNR " not supported on your arch", NR); \\ }) #endif