* [PATCH] staging:vt6656:baseband.h: fix function definition argument without identifier name issue @ 2016-10-13 11:50 Mikhail Golubev 2016-10-13 12:06 ` Greg KH 0 siblings, 1 reply; 8+ messages in thread From: Mikhail Golubev @ 2016-10-13 11:50 UTC (permalink / raw) To: forest; +Cc: gregkh, devel, linux-kernel, Mikhail Golubev Function definitions arguments should also have an identifier name as reported by checkpatch.pl. Signed-off-by: Mikhail Golubev <golubev.mikhail@gmail.com> --- drivers/staging/vt6656/baseband.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/staging/vt6656/baseband.h b/drivers/staging/vt6656/baseband.h index 7cc1387..fe1c25c 100644 --- a/drivers/staging/vt6656/baseband.h +++ b/drivers/staging/vt6656/baseband.h @@ -86,15 +86,15 @@ struct vnt_phy_field { unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type, unsigned int frame_length, u16 tx_rate); -void vnt_get_phy_field(struct vnt_private *, u32 frame_length, - u16 tx_rate, u8 pkt_type, struct vnt_phy_field *); - -void vnt_set_short_slot_time(struct vnt_private *); -void vnt_set_vga_gain_offset(struct vnt_private *, u8); -void vnt_set_antenna_mode(struct vnt_private *, u8); -int vnt_vt3184_init(struct vnt_private *); -void vnt_set_deep_sleep(struct vnt_private *); -void vnt_exit_deep_sleep(struct vnt_private *); -void vnt_update_pre_ed_threshold(struct vnt_private *, int scanning); +void vnt_get_phy_field(struct vnt_private *priv, u32 frame_length, + u16 tx_rate, u8 pkt_type, struct vnt_phy_field *phy); + +void vnt_set_short_slot_time(struct vnt_private *priv); +void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data); +void vnt_set_antenna_mode(struct vnt_private *priv, u8 antenna_mode); +int vnt_vt3184_init(struct vnt_private *priv); +void vnt_set_deep_sleep(struct vnt_private *priv); +void vnt_exit_deep_sleep(struct vnt_private *priv); +void vnt_update_pre_ed_threshold(struct vnt_private *priv, int scanning); #endif /* __BASEBAND_H__ */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] staging:vt6656:baseband.h: fix function definition argument without identifier name issue 2016-10-13 11:50 [PATCH] staging:vt6656:baseband.h: fix function definition argument without identifier name issue Mikhail Golubev @ 2016-10-13 12:06 ` Greg KH 2016-10-13 14:23 ` Mikhail Golubev 2016-10-14 8:58 ` [PATCH v2] " Mikhail Golubev 0 siblings, 2 replies; 8+ messages in thread From: Greg KH @ 2016-10-13 12:06 UTC (permalink / raw) To: Mikhail Golubev; +Cc: forest, devel, linux-kernel On Thu, Oct 13, 2016 at 02:50:18PM +0300, Mikhail Golubev wrote: > Function definitions arguments should also have an identifier name as reported by checkpatch.pl. Please wrap your changelog comments at 72 columns. > > Signed-off-by: Mikhail Golubev <golubev.mikhail@gmail.com> > --- > drivers/staging/vt6656/baseband.h | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/staging/vt6656/baseband.h b/drivers/staging/vt6656/baseband.h > index 7cc1387..fe1c25c 100644 > --- a/drivers/staging/vt6656/baseband.h > +++ b/drivers/staging/vt6656/baseband.h > @@ -86,15 +86,15 @@ struct vnt_phy_field { > unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type, > unsigned int frame_length, u16 tx_rate); > > -void vnt_get_phy_field(struct vnt_private *, u32 frame_length, > - u16 tx_rate, u8 pkt_type, struct vnt_phy_field *); > - > -void vnt_set_short_slot_time(struct vnt_private *); > -void vnt_set_vga_gain_offset(struct vnt_private *, u8); > -void vnt_set_antenna_mode(struct vnt_private *, u8); > -int vnt_vt3184_init(struct vnt_private *); > -void vnt_set_deep_sleep(struct vnt_private *); > -void vnt_exit_deep_sleep(struct vnt_private *); > -void vnt_update_pre_ed_threshold(struct vnt_private *, int scanning); > +void vnt_get_phy_field(struct vnt_private *priv, u32 frame_length, > + u16 tx_rate, u8 pkt_type, struct vnt_phy_field *phy); > + Really? Since when is this a coding style requirement? Don't use --strict, it's not worth it for stuff like this, there are far worse-off files in drivers/staging/ than this one. thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Re: [PATCH] staging:vt6656:baseband.h: fix function definition argument without identifier name issue 2016-10-13 12:06 ` Greg KH @ 2016-10-13 14:23 ` Mikhail Golubev 2016-10-13 14:57 ` Greg KH 2016-10-14 8:58 ` [PATCH v2] " Mikhail Golubev 1 sibling, 1 reply; 8+ messages in thread From: Mikhail Golubev @ 2016-10-13 14:23 UTC (permalink / raw) To: Greg KH; +Cc: forest, devel, linux-kernel On Thu, Oct 13, 2016 at 02:06:02PM +0200, Greg KH wrote: > On Thu, Oct 13, 2016 at 02:50:18PM +0300, Mikhail Golubev wrote: > > Function definitions arguments should also have an identifier name as reported by checkpatch.pl. > > Please wrap your changelog comments at 72 columns. > > > > > Signed-off-by: Mikhail Golubev <golubev.mikhail@gmail.com> > > --- > > drivers/staging/vt6656/baseband.h | 20 ++++++++++---------- > > 1 file changed, 10 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/staging/vt6656/baseband.h b/drivers/staging/vt6656/baseband.h > > index 7cc1387..fe1c25c 100644 > > --- a/drivers/staging/vt6656/baseband.h > > +++ b/drivers/staging/vt6656/baseband.h > > @@ -86,15 +86,15 @@ struct vnt_phy_field { > > unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type, > > unsigned int frame_length, u16 tx_rate); > > > > -void vnt_get_phy_field(struct vnt_private *, u32 frame_length, > > - u16 tx_rate, u8 pkt_type, struct vnt_phy_field *); > > - > > -void vnt_set_short_slot_time(struct vnt_private *); > > -void vnt_set_vga_gain_offset(struct vnt_private *, u8); > > -void vnt_set_antenna_mode(struct vnt_private *, u8); > > -int vnt_vt3184_init(struct vnt_private *); > > -void vnt_set_deep_sleep(struct vnt_private *); > > -void vnt_exit_deep_sleep(struct vnt_private *); > > -void vnt_update_pre_ed_threshold(struct vnt_private *, int scanning); > > +void vnt_get_phy_field(struct vnt_private *priv, u32 frame_length, > > + u16 tx_rate, u8 pkt_type, struct vnt_phy_field *phy); > > + > > Really? Since when is this a coding style requirement? > This requirement is really new. It was proposed by Joe Perches at 26 Sep 2016: [PATCH] checkpatch: Add warning for unnamed function definition. Should this type of warnings be fixed here? > Don't use --strict, it's not worth it for stuff like this, there are far > worse-off files in drivers/staging/ than this one. > > thanks, > > greg k-h Best wishes, Mike. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Re: [PATCH] staging:vt6656:baseband.h: fix function definition argument without identifier name issue 2016-10-13 14:23 ` Mikhail Golubev @ 2016-10-13 14:57 ` Greg KH 2016-10-13 16:37 ` Joe Perches 0 siblings, 1 reply; 8+ messages in thread From: Greg KH @ 2016-10-13 14:57 UTC (permalink / raw) To: Mikhail Golubev, Joe Perches; +Cc: devel, forest, linux-kernel On Thu, Oct 13, 2016 at 05:23:45PM +0300, Mikhail Golubev wrote: > On Thu, Oct 13, 2016 at 02:06:02PM +0200, Greg KH wrote: > > On Thu, Oct 13, 2016 at 02:50:18PM +0300, Mikhail Golubev wrote: > > > Function definitions arguments should also have an identifier name as reported by checkpatch.pl. > > > > Please wrap your changelog comments at 72 columns. > > > > > > > > Signed-off-by: Mikhail Golubev <golubev.mikhail@gmail.com> > > > --- > > > drivers/staging/vt6656/baseband.h | 20 ++++++++++---------- > > > 1 file changed, 10 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/staging/vt6656/baseband.h b/drivers/staging/vt6656/baseband.h > > > index 7cc1387..fe1c25c 100644 > > > --- a/drivers/staging/vt6656/baseband.h > > > +++ b/drivers/staging/vt6656/baseband.h > > > @@ -86,15 +86,15 @@ struct vnt_phy_field { > > > unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type, > > > unsigned int frame_length, u16 tx_rate); > > > > > > -void vnt_get_phy_field(struct vnt_private *, u32 frame_length, > > > - u16 tx_rate, u8 pkt_type, struct vnt_phy_field *); > > > - > > > -void vnt_set_short_slot_time(struct vnt_private *); > > > -void vnt_set_vga_gain_offset(struct vnt_private *, u8); > > > -void vnt_set_antenna_mode(struct vnt_private *, u8); > > > -int vnt_vt3184_init(struct vnt_private *); > > > -void vnt_set_deep_sleep(struct vnt_private *); > > > -void vnt_exit_deep_sleep(struct vnt_private *); > > > -void vnt_update_pre_ed_threshold(struct vnt_private *, int scanning); > > > +void vnt_get_phy_field(struct vnt_private *priv, u32 frame_length, > > > + u16 tx_rate, u8 pkt_type, struct vnt_phy_field *phy); > > > + > > > > Really? Since when is this a coding style requirement? > > > > This requirement is really new. It was proposed by Joe Perches at 26 Sep 2016: > [PATCH] checkpatch: Add warning for unnamed function definition. > > Should this type of warnings be fixed here? Ugh, Joe, why did you add this option? What is it going to help out with? thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Re: [PATCH] staging:vt6656:baseband.h: fix function definition argument without identifier name issue 2016-10-13 14:57 ` Greg KH @ 2016-10-13 16:37 ` Joe Perches 2016-10-13 16:49 ` Greg KH 0 siblings, 1 reply; 8+ messages in thread From: Joe Perches @ 2016-10-13 16:37 UTC (permalink / raw) To: Greg KH, Mikhail Golubev, Andrew Morton; +Cc: devel, forest, linux-kernel On Thu, 2016-10-13 at 16:57 +0200, Greg KH wrote: > On Thu, Oct 13, 2016 at 05:23:45PM +0300, Mikhail Golubev wrote: > > On Thu, Oct 13, 2016 at 02:06:02PM +0200, Greg KH wrote: > > > On Thu, Oct 13, 2016 at 02:50:18PM +0300, Mikhail Golubev wrote: > > > > Function definitions arguments should also have an identifier name as reported by checkpatch.pl. [] > > > > diff --git a/drivers/staging/vt6656/baseband.h b/drivers/staging/vt6656/baseband.h [] > > > > @@ -86,15 +86,15 @@ struct vnt_phy_field { > > > > unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type, > > > > unsigned int frame_length, u16 tx_rate);p > > > > > > > > -void vnt_get_phy_field(struct vnt_private *, u32 frame_length, > > > > - u16 tx_rate, u8 pkt_type, struct vnt_phy_field *); > > > > - > > > > -void vnt_set_short_slot_time(struct vnt_private *); > > > > -void vnt_set_vga_gain_offset(struct vnt_private *, u8); > > > > -void vnt_set_antenna_mode(struct vnt_private *, u8); > > > > -int vnt_vt3184_init(struct vnt_private *); > > > > -void vnt_set_deep_sleep(struct vnt_private *); > > > > -void vnt_exit_deep_sleep(struct vnt_private *); > > > > -void vnt_update_pre_ed_threshold(struct vnt_private *, int scanning); > > > > +void vnt_get_phy_field(struct vnt_private *priv, u32 frame_length, > > > > + u16 tx_rate, u8 pkt_type, struct vnt_phy_field *phy); > > > > + > > > Really? Since when is this a coding style requirement? > > This requirement is really new. It was proposed by Joe Perches at 26 Sep 2016: > > [PATCH] checkpatch: Add warning for unnamed function definition. > > > > Should this type of warnings be fixed here? > Ugh, Joe, why did you add this option? 1. Most all kernel prototypes use named arguments. 2. It helps make header files easier to read/lookup with grep. int func(int, int, int) vs int func(int weight, int density, int mass) which is easier for humans to use? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Re: [PATCH] staging:vt6656:baseband.h: fix function definition argument without identifier name issue 2016-10-13 16:37 ` Joe Perches @ 2016-10-13 16:49 ` Greg KH 2016-10-13 16:56 ` Joe Perches 0 siblings, 1 reply; 8+ messages in thread From: Greg KH @ 2016-10-13 16:49 UTC (permalink / raw) To: Joe Perches; +Cc: Mikhail Golubev, Andrew Morton, devel, forest, linux-kernel On Thu, Oct 13, 2016 at 09:37:23AM -0700, Joe Perches wrote: > On Thu, 2016-10-13 at 16:57 +0200, Greg KH wrote: > > On Thu, Oct 13, 2016 at 05:23:45PM +0300, Mikhail Golubev wrote: > > > On Thu, Oct 13, 2016 at 02:06:02PM +0200, Greg KH wrote: > > > > On Thu, Oct 13, 2016 at 02:50:18PM +0300, Mikhail Golubev wrote: > > > > > Function definitions arguments should also have an identifier name as reported by checkpatch.pl. > [] > > > > > diff --git a/drivers/staging/vt6656/baseband.h b/drivers/staging/vt6656/baseband.h > [] > > > > > @@ -86,15 +86,15 @@ struct vnt_phy_field { > > > > > unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type, > > > > > unsigned int frame_length, u16 tx_rate);p > > > > > > > > > > -void vnt_get_phy_field(struct vnt_private *, u32 frame_length, > > > > > - u16 tx_rate, u8 pkt_type, struct vnt_phy_field *); > > > > > - > > > > > -void vnt_set_short_slot_time(struct vnt_private *); > > > > > -void vnt_set_vga_gain_offset(struct vnt_private *, u8); > > > > > -void vnt_set_antenna_mode(struct vnt_private *, u8); > > > > > -int vnt_vt3184_init(struct vnt_private *); > > > > > -void vnt_set_deep_sleep(struct vnt_private *); > > > > > -void vnt_exit_deep_sleep(struct vnt_private *); > > > > > -void vnt_update_pre_ed_threshold(struct vnt_private *, int scanning); > > > > > +void vnt_get_phy_field(struct vnt_private *priv, u32 frame_length, > > > > > + u16 tx_rate, u8 pkt_type, struct vnt_phy_field *phy); > > > > > + > > > > Really? Since when is this a coding style requirement? > > > This requirement is really new. It was proposed by Joe Perches at 26 Sep 2016: > > > [PATCH] checkpatch: Add warning for unnamed function definition. > > > > > > Should this type of warnings be fixed here? > > Ugh, Joe, why did you add this option? > > 1. Most all kernel prototypes use named arguments. > 2. It helps make header files easier to read/lookup with grep. > > int func(int, int, int) > vs > int func(int weight, int density, int mass) > > which is easier for humans to use? Yes, which is why I use that format, but is it something we are now going to suddenly require? Also, this is going to take a lot more work to review patches like this, to match up the variable names to ensure that the developer got it right... thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Re: [PATCH] staging:vt6656:baseband.h: fix function definition argument without identifier name issue 2016-10-13 16:49 ` Greg KH @ 2016-10-13 16:56 ` Joe Perches 0 siblings, 0 replies; 8+ messages in thread From: Joe Perches @ 2016-10-13 16:56 UTC (permalink / raw) To: Greg KH, Julia Lawall Cc: Mikhail Golubev, Andrew Morton, devel, forest, linux-kernel On Thu, 2016-10-13 at 18:49 +0200, Greg KH wrote: > On Thu, Oct 13, 2016 at 09:37:23AM -0700, Joe Perches wrote: > > On Thu, 2016-10-13 at 16:57 +0200, Greg KH wrote: > > > On Thu, Oct 13, 2016 at 05:23:45PM +0300, Mikhail Golubev wrote: > > > > On Thu, Oct 13, 2016 at 02:06:02PM +0200, Greg KH wrote: > > > > > On Thu, Oct 13, 2016 at 02:50:18PM +0300, Mikhail Golubev wrote: > > > > > > Function definitions arguments should also have an identifier name as reported by checkpatch.pl. > > > > [] > > > > > > diff --git a/drivers/staging/vt6656/baseband.h b/drivers/staging/vt6656/baseband.h > > > > [] > > > > > > @@ -86,15 +86,15 @@ struct vnt_phy_field { > > > > > > unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type, > > > > > > unsigned int frame_length, u16 tx_rate);p > > > > > > > > > > > > -void vnt_get_phy_field(struct vnt_private *, u32 frame_length, > > > > > > - u16 tx_rate, u8 pkt_type, struct vnt_phy_field *); > > > > > > - > > > > > > -void vnt_set_short_slot_time(struct vnt_private *); > > > > > > -void vnt_set_vga_gain_offset(struct vnt_private *, u8); > > > > > > -void vnt_set_antenna_mode(struct vnt_private *, u8); > > > > > > -int vnt_vt3184_init(struct vnt_private *); > > > > > > -void vnt_set_deep_sleep(struct vnt_private *); > > > > > > -void vnt_exit_deep_sleep(struct vnt_private *); > > > > > > -void vnt_update_pre_ed_threshold(struct vnt_private *, int scanning); > > > > > > +void vnt_get_phy_field(struct vnt_private *priv, u32 frame_length, > > > > > > + u16 tx_rate, u8 pkt_type, struct vnt_phy_field *phy); > > > > > > + > > > > > > > > > > Really? Since when is this a coding style requirement? > > > > > > > > This requirement is really new. It was proposed by Joe Perches at 26 Sep 2016: > > > > [PATCH] checkpatch: Add warning for unnamed function definition. > > > > > > > > Should this type of warnings be fixed here? > > > > > > Ugh, Joe, why did you add this option? > > > > > > 1. Most all kernel prototypes use named arguments. > > 2. It helps make header files easier to read/lookup with grep. > > > > int func(int, int, int) > > vs > > int func(int weight, int density, int mass) > > > > which is easier for humans to use? > > > Yes, which is why I use that format, but is it something we are now > going to suddenly require? > > Also, this is going to take a lot more work to review patches like this, > to match up the variable names to ensure that the developer got it > right... Coccinelle to the rescue... http://www.spinics.net/lists/linux-kernel-janitors/msg28450.html Julia Lawall wrote a script for that. Unfortunately, it doesn't currently span .h and .c files --- @r@ identifier f; position p; type T, t; parameter list[n] ps; @@ T f@p(ps,t,...); @s@ identifier r.f,x; type r.T, r.t; parameter list[r.n] ps; @@ T f(ps,t x,...) { ... } @@ identifier r.f, s.x; position r.p; type r.T, r.t; parameter list[r.n] ps; @@ T f@p(ps,t + x ,...); ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] staging:vt6656:baseband.h: fix function definition argument without identifier name issue 2016-10-13 12:06 ` Greg KH 2016-10-13 14:23 ` Mikhail Golubev @ 2016-10-14 8:58 ` Mikhail Golubev 1 sibling, 0 replies; 8+ messages in thread From: Mikhail Golubev @ 2016-10-14 8:58 UTC (permalink / raw) To: gregkh; +Cc: forest, devel, linux-kernel, Mikhail Golubev Function definitions arguments should also have an identifier name as reported by checkpatch.pl. Signed-off-by: Mikhail Golubev <golubev.mikhail@gmail.com> --- Changes in v2: - Make the changelog comment wrapped at 72 columns. drivers/staging/vt6656/baseband.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/staging/vt6656/baseband.h b/drivers/staging/vt6656/baseband.h index 7cc1387..fe1c25c 100644 --- a/drivers/staging/vt6656/baseband.h +++ b/drivers/staging/vt6656/baseband.h @@ -86,15 +86,15 @@ struct vnt_phy_field { unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type, unsigned int frame_length, u16 tx_rate); -void vnt_get_phy_field(struct vnt_private *, u32 frame_length, - u16 tx_rate, u8 pkt_type, struct vnt_phy_field *); - -void vnt_set_short_slot_time(struct vnt_private *); -void vnt_set_vga_gain_offset(struct vnt_private *, u8); -void vnt_set_antenna_mode(struct vnt_private *, u8); -int vnt_vt3184_init(struct vnt_private *); -void vnt_set_deep_sleep(struct vnt_private *); -void vnt_exit_deep_sleep(struct vnt_private *); -void vnt_update_pre_ed_threshold(struct vnt_private *, int scanning); +void vnt_get_phy_field(struct vnt_private *priv, u32 frame_length, + u16 tx_rate, u8 pkt_type, struct vnt_phy_field *phy); + +void vnt_set_short_slot_time(struct vnt_private *priv); +void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data); +void vnt_set_antenna_mode(struct vnt_private *priv, u8 antenna_mode); +int vnt_vt3184_init(struct vnt_private *priv); +void vnt_set_deep_sleep(struct vnt_private *priv); +void vnt_exit_deep_sleep(struct vnt_private *priv); +void vnt_update_pre_ed_threshold(struct vnt_private *priv, int scanning); #endif /* __BASEBAND_H__ */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-10-14 8:58 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-10-13 11:50 [PATCH] staging:vt6656:baseband.h: fix function definition argument without identifier name issue Mikhail Golubev 2016-10-13 12:06 ` Greg KH 2016-10-13 14:23 ` Mikhail Golubev 2016-10-13 14:57 ` Greg KH 2016-10-13 16:37 ` Joe Perches 2016-10-13 16:49 ` Greg KH 2016-10-13 16:56 ` Joe Perches 2016-10-14 8:58 ` [PATCH v2] " Mikhail Golubev
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox