| Message ID | 20220906054612.9790-1-pvorel@suse.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] checkpatch: Ignore warnings irrelevant in userspace | expand |
Hi! > * EMBEDDED_FILENAME > fanotify.h:15: WARNING: It's generally not useful to have the filename in the file > on #include <sys/fanotify.h> in fanotify.h > > * ENOSYS > fanotify.h:26: WARNING: ENOSYS means 'invalid syscall nr' and nothing else > on if (errno == ENOSYS) > > * NEW_TYPEDEFS > fanotify.h:180: WARNING: do not add new typedefs > on typedef struct { I'm not 100% sure about the NEW_TYPEDEFS check, that one is mostly right. The later two are fine.
> Hi! > > * EMBEDDED_FILENAME > > fanotify.h:15: WARNING: It's generally not useful to have the filename in the file > > on #include <sys/fanotify.h> in fanotify.h > > * ENOSYS > > fanotify.h:26: WARNING: ENOSYS means 'invalid syscall nr' and nothing else > > on if (errno == ENOSYS) > > * NEW_TYPEDEFS > > fanotify.h:180: WARNING: do not add new typedefs > > on typedef struct { > I'm not 100% sure about the NEW_TYPEDEFS check, that one is mostly > right. The later two are fine. FYI the error is from fanotify.h (kind of lapi file for fanotify: #ifndef __kernel_fsid_t typedef struct { int val[2]; } lapi_fsid_t; #define __kernel_fsid_t lapi_fsid_t #endif /* __kernel_fsid_t */ which we added in b8aebc835 ("fanotify: Fix missing __kernel_fsid_t definition") "Instead of including <asm/posix_types.h> where it's defined we just define the missing bit." (fix for musl). But if you prefer to keep this check, I'm ok to merge it without it. The long term solution could be to add variable to Makefile to pass extra parameters, e.g.: check_fanotify.h: CHECKPATCH_IGNORE += NEW_TYPEDEFS Kind regards, Petr
Hi! > FYI the error is from fanotify.h (kind of lapi file for fanotify: < > #ifndef __kernel_fsid_t > typedef struct { > int val[2]; > } lapi_fsid_t; > #define __kernel_fsid_t lapi_fsid_t > #endif /* __kernel_fsid_t */ > > which we added in b8aebc835 ("fanotify: Fix missing __kernel_fsid_t definition") > "Instead of including <asm/posix_types.h> where it's defined we just > define the missing bit." (fix for musl). I'm aware of that, and while typedef is mostly wrong there are a few places where it's required. > But if you prefer to keep this check, I'm ok to merge it without it. > > The long term solution could be to add variable to Makefile to pass extra > parameters, e.g.: > check_fanotify.h: CHECKPATCH_IGNORE += NEW_TYPEDEFS Case by case decisions like this for typedef sounds better to me.
> Hi! > > FYI the error is from fanotify.h (kind of lapi file for fanotify: > < > > #ifndef __kernel_fsid_t > > typedef struct { > > int val[2]; > > } lapi_fsid_t; > > #define __kernel_fsid_t lapi_fsid_t > > #endif /* __kernel_fsid_t */ > > which we added in b8aebc835 ("fanotify: Fix missing __kernel_fsid_t definition") > > "Instead of including <asm/posix_types.h> where it's defined we just > > define the missing bit." (fix for musl). > I'm aware of that, and while typedef is mostly wrong there are a few > places where it's required. > > But if you prefer to keep this check, I'm ok to merge it without it. > > The long term solution could be to add variable to Makefile to pass extra > > parameters, e.g.: > > check_fanotify.h: CHECKPATCH_IGNORE += NEW_TYPEDEFS > Case by case decisions like this for typedef sounds better to me. +1 (TODO after the release). Thanks for your time! Kind regards, Petr
Hi, Petr Vorel <pvorel@suse.cz> writes: >> Hi! >> > FYI the error is from fanotify.h (kind of lapi file for fanotify: >> < >> > #ifndef __kernel_fsid_t >> > typedef struct { >> > int val[2]; >> > } lapi_fsid_t; >> > #define __kernel_fsid_t lapi_fsid_t >> > #endif /* __kernel_fsid_t */ > >> > which we added in b8aebc835 ("fanotify: Fix missing __kernel_fsid_t definition") >> > "Instead of including <asm/posix_types.h> where it's defined we just >> > define the missing bit." (fix for musl). > >> I'm aware of that, and while typedef is mostly wrong there are a few >> places where it's required. > >> > But if you prefer to keep this check, I'm ok to merge it without it. > >> > The long term solution could be to add variable to Makefile to pass extra >> > parameters, e.g.: >> > check_fanotify.h: CHECKPATCH_IGNORE += NEW_TYPEDEFS > >> Case by case decisions like this for typedef sounds better to me. > +1 (TODO after the release). > Thanks for your time! Don't forget this, I'm setting it changes requested in patchwork > > Kind regards, > Petr
diff --git a/include/mk/env_post.mk b/include/mk/env_post.mk index a00f31b08..184481f6c 100644 --- a/include/mk/env_post.mk +++ b/include/mk/env_post.mk @@ -94,7 +94,7 @@ CHECK_TARGETS ?= $(addprefix check-,$(notdir $(patsubst %.c,%,$(sort $(wildcar CHECK_TARGETS := $(filter-out $(addprefix check-, $(FILTER_OUT_MAKE_TARGETS)), $(CHECK_TARGETS)) CHECK_HEADER_TARGETS ?= $(addprefix check-,$(notdir $(sort $(wildcard $(abs_srcdir)/*.h)))) CHECK ?= $(abs_top_srcdir)/tools/sparse/sparse-ltp -CHECK_NOFLAGS ?= $(abs_top_srcdir)/scripts/checkpatch.pl -f --no-tree --terse --no-summary --ignore CONST_STRUCT,VOLATILE,SPLIT_STRING +CHECK_NOFLAGS ?= $(abs_top_srcdir)/scripts/checkpatch.pl -f --no-tree --terse --no-summary --ignore CONST_STRUCT,VOLATILE,SPLIT_STRING,EMBEDDED_FILENAME,ENOSYS,NEW_TYPEDEFS SHELL_CHECK ?= $(abs_top_srcdir)/scripts/checkbashisms.pl --force --extra SHELL_CHECK_TARGETS ?= $(addprefix check-,$(notdir $(sort $(wildcard $(abs_srcdir)/*.sh))))
* EMBEDDED_FILENAME fanotify.h:15: WARNING: It's generally not useful to have the filename in the file on #include <sys/fanotify.h> in fanotify.h * ENOSYS fanotify.h:26: WARNING: ENOSYS means 'invalid syscall nr' and nothing else on if (errno == ENOSYS) * NEW_TYPEDEFS fanotify.h:180: WARNING: do not add new typedefs on typedef struct { Signed-off-by: Petr Vorel <pvorel@suse.cz> --- include/mk/env_post.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)