2015-04-28 14:15:23

by Valentin Rothberg

[permalink] [raw]
Subject: [PATCH] drivers/scsi/arm/acornscsi.c: rename CONFIG_ACORNSCSI_CONSTANTS

CONFIG_ACORNSCSI_CONSTANTS is a file local CPP identifier and thereby
violates the naming convention of Kconfig options in Make and CPP
syntax; only Kconfig options should carry the 'CONFIG_' prefix.

This patch removes the 'CONFIG_' prefix to apply to this convention and
to make static analysis tools happy.

Signed-off-by: Valentin Rothberg <[email protected]>
---
I detected this issue with ./scripts/checkkconfigsymbols.py
---
drivers/scsi/arm/acornscsi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/arm/acornscsi.c b/drivers/scsi/arm/acornscsi.c
index deaaf84989cd..ce764c3ed99c 100644
--- a/drivers/scsi/arm/acornscsi.c
+++ b/drivers/scsi/arm/acornscsi.c
@@ -96,7 +96,7 @@
* Define this if you want to have verbose explanation of SCSI
* status/messages.
*/
-#undef CONFIG_ACORNSCSI_CONSTANTS
+#undef ACORNSCSI_CONSTANTS
/*
* Define this if you want to use the on board DMAC [don't remove this option]
* If not set, then use PIO mode (not currently supported).
@@ -399,7 +399,7 @@ void acornscsi_resetcard(AS_Host *host)
/*=============================================================================================
* Utility routines (eg. debug)
*/
-#ifdef CONFIG_ACORNSCSI_CONSTANTS
+#ifdef ACORNSCSI_CONSTANTS
static char *acornscsi_interrupttype[] = {
"rst", "suc", "p/a", "3",
"term", "5", "6", "7",
@@ -477,7 +477,7 @@ void print_scsi_status(unsigned int ssr)
static
void print_sbic_status(int asr, int ssr, int cmdphase)
{
-#ifdef CONFIG_ACORNSCSI_CONSTANTS
+#ifdef ACORNSCSI_CONSTANTS
printk("sbic: %c%c%c%c%c%c ",
asr & ASR_INT ? 'I' : 'i',
asr & ASR_LCI ? 'L' : 'l',
--
2.1.4


2015-04-28 19:10:30

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH] drivers/scsi/arm/acornscsi.c: rename CONFIG_ACORNSCSI_CONSTANTS

On Tue, 2015-04-28 at 16:15 +0200, Valentin Rothberg wrote:
> CONFIG_ACORNSCSI_CONSTANTS is a file local CPP identifier and thereby
> violates the naming convention of Kconfig options in Make and CPP
> syntax; only Kconfig options should carry the 'CONFIG_' prefix.
>
> This patch removes the 'CONFIG_' prefix to apply to this convention and
> to make static analysis tools happy.

Will the Erlangen bot still spot ACORNSCSI_CONSTANTS as a potential
issue?


Paul Bolle

2015-04-28 19:26:55

by Valentin Rothberg

[permalink] [raw]
Subject: Re: [PATCH] drivers/scsi/arm/acornscsi.c: rename CONFIG_ACORNSCSI_CONSTANTS

Hi Paul,

On Tue, Apr 28, 2015 at 9:10 PM, Paul Bolle <[email protected]> wrote:
> On Tue, 2015-04-28 at 16:15 +0200, Valentin Rothberg wrote:
>> CONFIG_ACORNSCSI_CONSTANTS is a file local CPP identifier and thereby
>> violates the naming convention of Kconfig options in Make and CPP
>> syntax; only Kconfig options should carry the 'CONFIG_' prefix.
>>
>> This patch removes the 'CONFIG_' prefix to apply to this convention and
>> to make static analysis tools happy.
>
> Will the Erlangen bot still spot ACORNSCSI_CONSTANTS as a potential
> issue?

No, undertaker-checkpatch won't complain about this. There are
thousands of such cases (i.e., without CONFIG_ prefix) around in the
code (mostly #ifdef DEBUG). But most of them are intentionally dead
or related to debugging, so they are ignored to avoid having false
positives.

Regards,
Valentin

> Paul Bolle
>

2015-04-28 19:34:48

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH] drivers/scsi/arm/acornscsi.c: rename CONFIG_ACORNSCSI_CONSTANTS

Hi Valentin,

On Tue, 2015-04-28 at 21:26 +0200, Valentin Rothberg wrote:
> On Tue, Apr 28, 2015 at 9:10 PM, Paul Bolle <[email protected]> wrote:
> > Will the Erlangen bot still spot ACORNSCSI_CONSTANTS as a potential
> > issue?
>
> No, undertaker-checkpatch won't complain about this. There are
> thousands of such cases (i.e., without CONFIG_ prefix) around in the
> code (mostly #ifdef DEBUG). But most of them are intentionally dead
> or related to debugging, so they are ignored to avoid having false
> positives.

Well, in a few years time, once undertaker-checkpatch has stomped out
most of the faux Kconfig preprocessor checks, that might be an area to
cover too. Or is that issue, ie pointless preprocessor checks, harder
than one might naively think?

Thanks,


Paul Bolle

2015-04-28 19:58:18

by Valentin Rothberg

[permalink] [raw]
Subject: Re: [PATCH] drivers/scsi/arm/acornscsi.c: rename CONFIG_ACORNSCSI_CONSTANTS

On Tue, Apr 28, 2015 at 9:34 PM, Paul Bolle <[email protected]> wrote:
> Hi Valentin,
>
> On Tue, 2015-04-28 at 21:26 +0200, Valentin Rothberg wrote:
>> On Tue, Apr 28, 2015 at 9:10 PM, Paul Bolle <[email protected]> wrote:
>> > Will the Erlangen bot still spot ACORNSCSI_CONSTANTS as a potential
>> > issue?
>>
>> No, undertaker-checkpatch won't complain about this. There are
>> thousands of such cases (i.e., without CONFIG_ prefix) around in the
>> code (mostly #ifdef DEBUG). But most of them are intentionally dead
>> or related to debugging, so they are ignored to avoid having false
>> positives.
>
> Well, in a few years time, once undertaker-checkpatch has stomped out
> most of the faux Kconfig preprocessor checks, that might be an area to
> cover too. Or is that issue, ie pointless preprocessor checks, harder
> than one might naively think?

To give a number from Linus' tree today: 4706 of such unprefixed dead
and undead #ifdefs and 940 'real' ones. Most of them are intentional
-- this doesn't mean that it's not a problem. Personally, I don't
like to have code around that cannot at least be easily test compiled;
we manually need to (un)define the identifiers.

I like your proposal. For symbolic issues, we could even put that
directly in checkkconfigsymbols.py, so a big part of the problem could
be solved directly with Kernel tools. But I really hope that it won't
take a few years until we get there : )

Valentin

> Thanks,
>
>
> Paul Bolle
>

2015-05-06 10:39:05

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH] drivers/scsi/arm/acornscsi.c: rename CONFIG_ACORNSCSI_CONSTANTS

On Tue, Apr 28, 2015 at 04:15:11PM +0200, Valentin Rothberg wrote:
> CONFIG_ACORNSCSI_CONSTANTS is a file local CPP identifier and thereby
> violates the naming convention of Kconfig options in Make and CPP
> syntax; only Kconfig options should carry the 'CONFIG_' prefix.
>
> This patch removes the 'CONFIG_' prefix to apply to this convention and
> to make static analysis tools happy.
>
> Signed-off-by: Valentin Rothberg <[email protected]>

As you're merely renaming all instances of this:

Acked-by: Russell King <[email protected]>

Thanks.

--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.