From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753881AbdDJPZy (ORCPT ); Mon, 10 Apr 2017 11:25:54 -0400 Received: from relay2-d.mail.gandi.net ([217.70.183.194]:43657 "EHLO relay2-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752752AbdDJPZx (ORCPT ); Mon, 10 Apr 2017 11:25:53 -0400 X-Originating-IP: 81.57.179.208 Subject: Re: [PATCH] Staging: comedidev.h comedi_lrange should be const struct References: <20170409194120.GA23199@Leliel> To: Greg KH Cc: devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, ybalrid@ybalrid.info From: Arthur Brainville X-Forwarded-Message-Id: <20170409194120.GA23199@Leliel> Message-ID: <50dc3790-104e-5771-170f-3f454c7a83fc@ybalrid.info> Date: Mon, 10 Apr 2017 17:25:49 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170409194120.GA23199@Leliel> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? Like I said here the `const` qualifier doen't change anything, you could create a `struct comedi_lrange` variable with some initialization, and change it later, it will not be const, unless declared so at that point. It was just to make this file pass the checkpatch perl script without warning, but it makes the staging comedi driver compile with warnings. It doesn't change anything "functionally", if you see what I mean.