Re: [Outreachy kernel] [PATCH] staging: most: hdm-usb: add comment for struct mutex definition

[Joe Perches <joe@perches.com>]

On Sat, 2017-09-16 at 11:35 +0200, Julia Lawall wrote:
> 
> On Sat, 16 Sep 2017, Joe Perches wrote:
> 
> > On Sat, 2017-09-16 at 10:23 +0200, Julia Lawall wrote:
> > > On Sat, 16 Sep 2017, Aastha Gupta wrote:
> > > > On Sat, Sep 16, 2017 at 1:18 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> > > > > On Sat, 16 Sep 2017, Aastha Gupta wrote:
> > > > > 
> > > > > > This patch fixes following checkpatch.pl checks:
> > > > > > CHECK: struct mutex definition without comment
> > 
> > []
> > > > > > diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c b/drivers/staging/most/hdm-usb/hdm_usb.c
> > 
> > []
> > > > > > @@ -122,7 +122,7 @@ struct most_dev {
> > > > > >       bool is_channel_healthy[MAX_NUM_ENDPOINTS];
> > > > > >       struct clear_hold_work clear_work[MAX_NUM_ENDPOINTS];
> > > > > >       struct usb_anchor *busy_urbs;
> > > > > > -     struct mutex io_mutex;
> > > > > > +     struct mutex io_mutex; /* synchronize I/O with disconnect */
> > > > > 
> > > > > Why did you choose this comment?
> > > > > 
> > > > > julia
> > > > 
> > > > The use of this struct mutex was mentioned before in the code.
> > > 
> > > I see that it is just in the doc of the same structure.  I'm not sure that
> > > a double comment is needed in this case. I wonder if checkpatch
> > > should/could be extended to address this.
> > 
> > I think that's not feasible.
> > 
> > checkpatch is a patch context based check and wouldn't
> > necessarily know that the mutex is documented elsewhere.
> 
> OK.  It doesn't keep any history?  The kerneldoc is before the structure
> field declaration.

checkpatch reads and stores its input.

checkpatch is, with very few exceptions, a line based
regex scanner.

An input patch context is typically 3 lines above
and below modified content.

Whatever context outside of that +/- 3 is unavailable.

Kernel-doc comments are not parsed by checkpatch as
anything special.

checkpatch code that looks for specific comments looks
only at whether or not a particular block has a comment
at all, not the specific content of the comment.

The checkpatch function test is ctx_has_comment

You are welcome to try to expand the code but I think
there are better possible tools for this.