linux-kernel - Re: [BUG ?] checkpatch.pl rejects as error something I think it ought to be allow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20110222090716.GH2809@shadowen.org>
Date:	Tue, 22 Feb 2011 09:07:16 +0000
From:	Andy Whitcroft <apw@...onical.com>
To:	Corey Ashford <cjashfor@...ux.vnet.ibm.com>
Cc:	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [BUG ?] checkpatch.pl rejects as error something I think it
 ought to be allow

On Mon, Feb 21, 2011 at 03:49:33PM -0800, Corey Ashford wrote:
> On 02/21/2011 03:40 PM, Andy Whitcroft wrote:
> >On Mon, Feb 21, 2011 at 03:28:02PM -0800, Corey Ashford wrote:
> >>Hi,
> >>
> >>I have a piece of code where I have two constants defined as follows:
> >>
> >>	static const unsigned long polling_interval_sec = 1;
> >>	static const unsigned long polling_interval_ns = 0;
> >>
> >>Now, it's clear to me that I want these two values to have the
> >>keywords const and static.  I could use a #define here, but const
> >>static seemed cleaner to me.
> >>
> >>When I run checkpatch.pl across this code, I get this error:
> >>
> >>ERROR: do not initialise statics to 0 or NULL.
> >>
> >>I think the problem here is that another case is needed for "static
> >>const" that does allow 0.
> >>
> >>What do you think?
> >>
> >>Thanks for your consideration,
> >
> >The warning is intended to tell you that the = 0 is unnecessary.  Any
> >static is 0 by default I believe.  At some point the addition of the 0
> >would move the value from the bss to the data segment bloating the code.
> >This may no longer be true.
> 
> OK, but that means I'd have to have a declaration like this, which
> looks quite odd to me:
> 
> 	static const poll_interval_ns; /* = 0 */
> 
> I don't think that is preferable to this:
> 
> 	static const poll_interval_ns = 0;

Well I'd not bother with the comment as that is defined behaviour.  But I
suspect that gcc has gained the smarts to not break these and perhaps the
check is long in the tooth.  As for this patch, as checkpatch is intended
to be an advisory helper you are free to ignore its recommendations,
just expect to be asked about it by any human reviewers.

-apw
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/