* [PATCH] Staging: comedidev.h comedi_lrange should be const struct
@ 2017-04-09 14:28 Arthur Brainville (Ybalrid)
2017-04-09 16:22 ` kbuild test robot
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Arthur Brainville (Ybalrid) @ 2017-04-09 14:28 UTC (permalink / raw)
To: abbotti; +Cc: devel, linux-kernel, Arthur Brainville (Ybalrid)
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) <ybalrid@ybalrid.info>
---
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[];
};
--
2.12.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Staging: comedidev.h comedi_lrange should be const struct
2017-04-09 14:28 [PATCH] Staging: comedidev.h comedi_lrange should be const struct Arthur Brainville (Ybalrid)
@ 2017-04-09 16:22 ` kbuild test robot
2017-04-09 19:02 ` Greg KH
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2017-04-09 16:22 UTC (permalink / raw)
To: Arthur Brainville (Ybalrid)
Cc: kbuild-all, abbotti, devel, linux-kernel, Arthur Brainville (Ybalrid)
[-- Attachment #1: Type: text/plain, Size: 3473 bytes --]
Hi Arthur,
[auto build test WARNING on staging/staging-testing]
[also build test WARNING on v4.11-rc5 next-20170407]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Arthur-Brainville-Ybalrid/Staging-comedidev-h-comedi_lrange-should-be-const-struct/20170409-224503
config: x86_64-allyesdebian (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from drivers/staging/comedi/comedi_fops.c:34:0:
>> drivers/staging/comedi/comedidev.h:629:1: warning: useless type qualifier in empty declaration
};
^
vim +629 drivers/staging/comedi/comedidev.h
ed9eccbe8 David Schleef 2008-11-04 613 #define range_digital range_unipolar5
ed9eccbe8 David Schleef 2008-11-04 614
5459bc128 Ian Abbott 2015-09-21 615 /**
5459bc128 Ian Abbott 2015-09-21 616 * struct comedi_lrange - Describes a COMEDI range table
5459bc128 Ian Abbott 2015-09-21 617 * @length: Number of entries in the range table.
5459bc128 Ian Abbott 2015-09-21 618 * @range: Array of &struct comedi_krange, one for each range.
5459bc128 Ian Abbott 2015-09-21 619 *
5459bc128 Ian Abbott 2015-09-21 620 * Each element of @range[] describes the minimum and maximum physical range
5459bc128 Ian Abbott 2015-09-21 621 * range and the type of units. Typically, the type of unit is %UNIT_volt
5459bc128 Ian Abbott 2015-09-21 622 * (i.e. volts) and the minimum and maximum are in millionths of a volt.
5459bc128 Ian Abbott 2015-09-21 623 * There may also be a flag that indicates the minimum and maximum are merely
5459bc128 Ian Abbott 2015-09-21 624 * scale factors for an unknown, external reference.
5459bc128 Ian Abbott 2015-09-21 625 */
540e454c2 Arthur Brainville (Ybalrid 2017-04-09 626) const struct comedi_lrange {
ed9eccbe8 David Schleef 2008-11-04 627 int length;
3358a0ca2 Cheah Kok Cheong 2016-12-22 628 struct comedi_krange range[];
ed9eccbe8 David Schleef 2008-11-04 @629 };
ed9eccbe8 David Schleef 2008-11-04 630
42e558399 Ian Abbott 2015-09-21 631 /**
42e558399 Ian Abbott 2015-09-21 632 * comedi_range_is_bipolar() - Test if subdevice range is bipolar
42e558399 Ian Abbott 2015-09-21 633 * @s: COMEDI subdevice.
42e558399 Ian Abbott 2015-09-21 634 * @range: Index of range within a range table.
42e558399 Ian Abbott 2015-09-21 635 *
42e558399 Ian Abbott 2015-09-21 636 * Tests whether a range is bipolar by checking whether its minimum value
42e558399 Ian Abbott 2015-09-21 637 * is negative.
:::::: The code at line 629 was first introduced by commit
:::::: ed9eccbe8970f6eedc1b978c157caf1251a896d4 Staging: add comedi core
:::::: TO: David Schleef <ds@schleef.org>
:::::: CC: Greg Kroah-Hartman <gregkh@suse.de>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38598 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Staging: comedidev.h comedi_lrange should be const struct
2017-04-09 14:28 [PATCH] Staging: comedidev.h comedi_lrange should be const struct Arthur Brainville (Ybalrid)
2017-04-09 16:22 ` kbuild test robot
@ 2017-04-09 19:02 ` Greg KH
2017-04-10 2:51 ` kbuild test robot
2017-04-10 17:45 ` [PATCH] checkpatch: Avoid suggesting struct definitions should be const Joe Perches
3 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2017-04-09 19:02 UTC (permalink / raw)
To: Arthur Brainville (Ybalrid); +Cc: abbotti, devel, linux-kernel
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) <ybalrid@ybalrid.info>
> ---
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Staging: comedidev.h comedi_lrange should be const struct
2017-04-09 14:28 [PATCH] Staging: comedidev.h comedi_lrange should be const struct Arthur Brainville (Ybalrid)
2017-04-09 16:22 ` kbuild test robot
2017-04-09 19:02 ` Greg KH
@ 2017-04-10 2:51 ` kbuild test robot
2017-04-10 17:45 ` [PATCH] checkpatch: Avoid suggesting struct definitions should be const Joe Perches
3 siblings, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2017-04-10 2:51 UTC (permalink / raw)
To: Arthur Brainville (Ybalrid)
Cc: kbuild-all, abbotti, devel, linux-kernel, Arthur Brainville (Ybalrid)
Hi Arthur,
[auto build test WARNING on staging/staging-testing]
[also build test WARNING on v4.11-rc6 next-20170407]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Arthur-Brainville-Ybalrid/Staging-comedidev-h-comedi_lrange-should-be-const-struct/20170409-224503
config: x86_64-allmodconfig
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
make ARCH=x86_64 allmodconfig
make ARCH=x86_64
All warnings (new ones prefixed by >>):
vim +629 drivers/staging/comedi/drivers/../comedidev.h
ed9eccbe8 David Schleef 2008-11-04 613 #define range_digital range_unipolar5
ed9eccbe8 David Schleef 2008-11-04 614
5459bc128 Ian Abbott 2015-09-21 615 /**
5459bc128 Ian Abbott 2015-09-21 616 * struct comedi_lrange - Describes a COMEDI range table
5459bc128 Ian Abbott 2015-09-21 617 * @length: Number of entries in the range table.
5459bc128 Ian Abbott 2015-09-21 618 * @range: Array of &struct comedi_krange, one for each range.
5459bc128 Ian Abbott 2015-09-21 619 *
5459bc128 Ian Abbott 2015-09-21 620 * Each element of @range[] describes the minimum and maximum physical range
5459bc128 Ian Abbott 2015-09-21 621 * range and the type of units. Typically, the type of unit is %UNIT_volt
5459bc128 Ian Abbott 2015-09-21 622 * (i.e. volts) and the minimum and maximum are in millionths of a volt.
5459bc128 Ian Abbott 2015-09-21 623 * There may also be a flag that indicates the minimum and maximum are merely
5459bc128 Ian Abbott 2015-09-21 624 * scale factors for an unknown, external reference.
5459bc128 Ian Abbott 2015-09-21 625 */
540e454c2 Arthur Brainville (Ybalrid 2017-04-09 626) const struct comedi_lrange {
ed9eccbe8 David Schleef 2008-11-04 627 int length;
3358a0ca2 Cheah Kok Cheong 2016-12-22 628 struct comedi_krange range[];
ed9eccbe8 David Schleef 2008-11-04 @629 };
ed9eccbe8 David Schleef 2008-11-04 630
42e558399 Ian Abbott 2015-09-21 631 /**
42e558399 Ian Abbott 2015-09-21 632 * comedi_range_is_bipolar() - Test if subdevice range is bipolar
42e558399 Ian Abbott 2015-09-21 633 * @s: COMEDI subdevice.
42e558399 Ian Abbott 2015-09-21 634 * @range: Index of range within a range table.
42e558399 Ian Abbott 2015-09-21 635 *
42e558399 Ian Abbott 2015-09-21 636 * Tests whether a range is bipolar by checking whether its minimum value
42e558399 Ian Abbott 2015-09-21 637 * is negative.
:::::: The code at line 629 was first introduced by commit
:::::: ed9eccbe8970f6eedc1b978c157caf1251a896d4 Staging: add comedi core
:::::: TO: David Schleef <ds@schleef.org>
:::::: CC: Greg Kroah-Hartman <gregkh@suse.de>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] checkpatch: Avoid suggesting struct definitions should be const
2017-04-09 14:28 [PATCH] Staging: comedidev.h comedi_lrange should be const struct Arthur Brainville (Ybalrid)
` (2 preceding siblings ...)
2017-04-10 2:51 ` kbuild test robot
@ 2017-04-10 17:45 ` Joe Perches
2017-04-10 18:56 ` Arthur Brainville (Ybalrid)
3 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2017-04-10 17:45 UTC (permalink / raw)
To: Andrew Morton, Andy Whitcroft; +Cc: Greg KH, Arthur Brainville, linux-kernel
Many structs are generally used const and there is a known list
of these structs.
struct definitions should not be generally be declared const.
Add a test for the lack of an open brace immediately after the
struct to avoid definitions.
This avoids the false positive "struct foo should normally be const"
message only when the open brace is on the same line as the definition.
Signed-off-by: Joe Perches <joe@perches.com>
---
scripts/checkpatch.pl | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 089c974aa3a5..3a1cb9d7474e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6090,11 +6090,11 @@ sub process {
}
# check for various structs that are normally const (ops, kgdb, device_tree)
+# and avoid what seem like struct definitions 'struct foo {'
if ($line !~ /\bconst\b/ &&
- $line =~ /\bstruct\s+($const_structs)\b/) {
+ $line =~ /\bstruct\s+($const_structs)\b(?!\s*\{)/) {
WARN("CONST_STRUCT",
- "struct $1 should normally be const\n" .
- $herecurr);
+ "struct $1 should normally be const\n" . $herecurr);
}
# use of NR_CPUS is usually wrong
--
2.10.0.rc2.1.g053435c
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] checkpatch: Avoid suggesting struct definitions should be const
2017-04-10 17:45 ` [PATCH] checkpatch: Avoid suggesting struct definitions should be const Joe Perches
@ 2017-04-10 18:56 ` Arthur Brainville (Ybalrid)
0 siblings, 0 replies; 6+ messages in thread
From: Arthur Brainville (Ybalrid) @ 2017-04-10 18:56 UTC (permalink / raw)
To: Joe Perches; +Cc: Andrew Morton, Andy Whitcroft, Greg KH, linux-kernel
On Mon, Apr 10, 2017 at 10:45:55AM -0700, Joe Perches wrote:
> Many structs are generally used const and there is a known list
> of these structs.
>
> struct definitions should not be generally be declared const.
>
> Add a test for the lack of an open brace immediately after the
> struct to avoid definitions.
>
> This avoids the false positive "struct foo should normally be const"
> message only when the open brace is on the same line as the definition.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> scripts/checkpatch.pl | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 089c974aa3a5..3a1cb9d7474e 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -6090,11 +6090,11 @@ sub process {
> }
>
> # check for various structs that are normally const (ops, kgdb, device_tree)
> +# and avoid what seem like struct definitions 'struct foo {'
> if ($line !~ /\bconst\b/ &&
> - $line =~ /\bstruct\s+($const_structs)\b/) {
> + $line =~ /\bstruct\s+($const_structs)\b(?!\s*\{)/) {
> WARN("CONST_STRUCT",
> - "struct $1 should normally be const\n" .
> - $herecurr);
> + "struct $1 should normally be const\n" . $herecurr);
> }
>
> # use of NR_CPUS is usually wrong
> --
> 2.10.0.rc2.1.g053435c
>
Oh, right... Everything makes sence now!
That output from checkpatch was confusing. Well, at least for that
little newbie that got the weird idea to send a "fix" on some comedi
driver code (me).
(I have to say, that was an interesting experience, and if me sending
that patch made Joe Perches improve checkpatch a tiny little bit, it
wasn't *that* useless... I just hope I did not annoy people too much)
Obviusly, the patch I sent about that staging comedi driver should be
ignored. I'll be a bit more careful when looking at these thing, and
recheck with the coding-style part of the doc.
Regards,
Arthur Brainville
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-04-10 18:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-09 14:28 [PATCH] Staging: comedidev.h comedi_lrange should be const struct Arthur Brainville (Ybalrid)
2017-04-09 16:22 ` kbuild test robot
2017-04-09 19:02 ` Greg KH
2017-04-10 2:51 ` kbuild test robot
2017-04-10 17:45 ` [PATCH] checkpatch: Avoid suggesting struct definitions should be const Joe Perches
2017-04-10 18:56 ` Arthur Brainville (Ybalrid)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).