public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
* [LTP] [PATCH 1/1] checkpatch: Ignore warnings irrelevant in userspace
@ 2022-09-06  5:46 Petr Vorel
  2022-09-16 14:08 ` Cyril Hrubis
  0 siblings, 1 reply; 6+ messages in thread
From: Petr Vorel @ 2022-09-06  5:46 UTC (permalink / raw)
  To: ltp; +Cc: Richard Palethorpe

* 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(-)

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))))
 
-- 
2.37.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH 1/1] checkpatch: Ignore warnings irrelevant in userspace
  2022-09-06  5:46 [LTP] [PATCH 1/1] checkpatch: Ignore warnings irrelevant in userspace Petr Vorel
@ 2022-09-16 14:08 ` Cyril Hrubis
  2022-09-16 19:04   ` Petr Vorel
  0 siblings, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2022-09-16 14:08 UTC (permalink / raw)
  To: Petr Vorel; +Cc: Richard Palethorpe, ltp

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.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH 1/1] checkpatch: Ignore warnings irrelevant in userspace
  2022-09-16 14:08 ` Cyril Hrubis
@ 2022-09-16 19:04   ` Petr Vorel
  2022-09-19  7:49     ` Cyril Hrubis
  0 siblings, 1 reply; 6+ messages in thread
From: Petr Vorel @ 2022-09-16 19:04 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: Richard Palethorpe, ltp

> 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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH 1/1] checkpatch: Ignore warnings irrelevant in userspace
  2022-09-16 19:04   ` Petr Vorel
@ 2022-09-19  7:49     ` Cyril Hrubis
  2022-09-21 11:22       ` Petr Vorel
  0 siblings, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2022-09-19  7:49 UTC (permalink / raw)
  To: Petr Vorel; +Cc: Richard Palethorpe, ltp

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.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH 1/1] checkpatch: Ignore warnings irrelevant in userspace
  2022-09-19  7:49     ` Cyril Hrubis
@ 2022-09-21 11:22       ` Petr Vorel
  2022-10-17 10:41         ` Richard Palethorpe
  0 siblings, 1 reply; 6+ messages in thread
From: Petr Vorel @ 2022-09-21 11:22 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: Richard Palethorpe, ltp

> 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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH 1/1] checkpatch: Ignore warnings irrelevant in userspace
  2022-09-21 11:22       ` Petr Vorel
@ 2022-10-17 10:41         ` Richard Palethorpe
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Palethorpe @ 2022-10-17 10:41 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

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


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-10-17 10:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-06  5:46 [LTP] [PATCH 1/1] checkpatch: Ignore warnings irrelevant in userspace Petr Vorel
2022-09-16 14:08 ` Cyril Hrubis
2022-09-16 19:04   ` Petr Vorel
2022-09-19  7:49     ` Cyril Hrubis
2022-09-21 11:22       ` Petr Vorel
2022-10-17 10:41         ` Richard Palethorpe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox