From: Greg KH <gregkh@linuxfoundation.org>
To: Joe Perches <joe@perches.com>
Cc: Mikhail Golubev <golubev.mikhail@gmail.com>,
Andrew Morton <akpm@linux-foundation.org>,
devel@driverdev.osuosl.org, forest@alittletooquiet.net,
linux-kernel@vger.kernel.org
Subject: Re: Re: [PATCH] staging:vt6656:baseband.h: fix function definition argument without identifier name issue
Date: Thu, 13 Oct 2016 18:49:27 +0200 [thread overview]
Message-ID: <20161013164927.GA15120@kroah.com> (raw)
In-Reply-To: <1476376643.2164.14.camel@perches.com>
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
next prev parent reply other threads:[~2016-10-13 17:01 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2016-10-13 16:56 ` Joe Perches
2016-10-14 8:58 ` [PATCH v2] " Mikhail Golubev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161013164927.GA15120@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=akpm@linux-foundation.org \
--cc=devel@driverdev.osuosl.org \
--cc=forest@alittletooquiet.net \
--cc=golubev.mikhail@gmail.com \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.