From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753467AbdDJURt (ORCPT ); Mon, 10 Apr 2017 16:17:49 -0400 Received: from relay7-d.mail.gandi.net ([217.70.183.200]:57642 "EHLO relay7-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752850AbdDJURr (ORCPT ); Mon, 10 Apr 2017 16:17:47 -0400 X-Greylist: delayed 107278 seconds by postgrey-1.27 at vger.kernel.org; Mon, 10 Apr 2017 16:17:47 EDT Date: Mon, 10 Apr 2017 22:17:33 +0200 From: "Arthur Brainville (Ybalrid)" To: Greg KH Cc: devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Staging: comedidev.h comedi_lrange should be const struct Message-ID: <20170410201712.GA14367@Leliel> References: <20170409194120.GA23199@Leliel> <50dc3790-104e-5771-170f-3f454c7a83fc@ybalrid.info> <20170410200105.GA14258@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170410200105.GA14258@kroah.com> User-Agent: Mutt/1.8.0 (2017-02-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Apr 10, 2017 at 10:01:05PM +0200, Greg KH wrote: > On Mon, Apr 10, 2017 at 05:25:49PM +0200, Arthur Brainville wrote: > > On Sun, Apr 09, 2017 at 09:02:03PM +0200, Greg KH wrote: > > > On Sun, Apr 09, 2017 at 04:28:12PM +0200, Arthur Brainville (Ybalrid) wrote: > > > > According to checkpatch.pl, comedi_lrange should be declared as `const > > > > struct` instead of `struct` in driver/staging/comedidev.h > > > > > > > > Signed-off-by: Arthur Brainville (Ybalrid) > > > > --- > > > > drivers/staging/comedi/comedidev.h | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h > > > > index 1bb9986f865e..82df090783b5 100644 > > > > --- a/drivers/staging/comedi/comedidev.h > > > > +++ b/drivers/staging/comedi/comedidev.h > > > > @@ -623,7 +623,7 @@ extern const struct comedi_lrange range_unknown; > > > > * There may also be a flag that indicates the minimum and maximum are merely > > > > * scale factors for an unknown, external reference. > > > > */ > > > > -struct comedi_lrange { > > > > +const struct comedi_lrange { > > > > int length; > > > > struct comedi_krange range[]; > > > > }; > > > > > > Huh? Please explain how this change is correct. > > > > > > greg k-h > > > > First of all, thanks for taking the time to reply! > > Well, sorry if I did something wrong, I'm a newbie here, so let me > > explain: > > > > When checking a bunch of files with /script/checkpatch.pl -f on the > > staging directory, it gave me this output: > > > > > WARNING: struct comedi_lrange should normally be const > > > #626: FILE: drivers/staging/comedi/comedidev.h:626: > > > +struct comedi_lrange { > > > > > > total: 0 errors, 1 warnings, 6 checks, 1043 lines checked > > > > > > NOTE: For some of the reported defects, checkpatch may be able to > > > mechanically convert to the typical style using --fix or > > > --fix-inplace. > > > > For this specific header. > > > > Declaring it as const struct fixes that coding style warning, and > > doesn't break the build but, AFAIK isn't actually meaningfull since it's > > declaring a struct, but not any variable of type "comedi_lrange" > > at this point. And it built fine. > > I was watching an old talk (from you) about how you can send a patch to > > correct this kind of little things, so I tried. > > > > Looking further into it, it seems that this change makes GCC trigger the > > following warning in any file that #include it : > > > In file included from drivers/staging/comedi//comedi_usb.h:24:0, > > > from drivers/staging/comedi//comedi_usb.c:21: > > > drivers/staging/comedi//comedidev.h:629:1: warning: > > > useless type qualifier in empty declaration > > > }; > > > > So, probably not good? > > You should always test-build your patches, and they can never add new > kernel warnings to the build, that's not allowed at all. So please do > that next time, it should have shown you that this was not a good > change. > > And as you have found out, checkpatch.pl is a dumb tool, you still have > to think when you use it. And if you know C, you will know that the > change you made makes no sense at all :) > > thanks, > > greg k-h Lesson learned! (It did build. But well, I don't know what I was thinking not paying attentions to theses warning. Shame on me!) Thank you again for your patience Greg :) Arthur Brainville