linux-kernel - Re: [PATCH 08/14] net: axienet: Removed checkpatch errors/warnings

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] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 13 Feb 2014 07:51:47 -0800
From:	Joe Perches <joe@...ches.com>
To:	Michal Simek <michal.simek@...inx.com>
Cc:	netdev@...r.kernel.org, Srikanth Thokala <sthokal@...inx.com>,
	Srikanth Thokala <srikanth.thokala@...inx.com>,
	Michal Simek <monstr@...str.eu>,
	Anirudha Sarangi <anirudh@...inx.com>,
	John Linn <John.Linn@...inx.com>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 08/14] net: axienet: Removed checkpatch errors/warnings

On Thu, 2014-02-13 at 08:19 +0100, Michal Simek wrote:
> On 02/13/2014 01:31 AM, Joe Perches wrote:
> > On Wed, 2014-02-12 at 16:55 +0100, Michal Simek wrote:

Hi again Michal.

> >> +		netdev_warn(lp->ndev,
> >> +			 "Could not find clock ethernet controller property.");
> > 
> > here too. (and alignment)
> 
> This is problematic. I would like to keep 80 char limits and keeping
> this align just break it. That's why I was using tab alignment.
> Probably the solution is just to shorten message.

(overly long, tiresomely trivial stuff below)

Your choice.  I'm not an 80 column zealot but
please don't shorten the message just to fit
80 columns if it impacts intelligibility.

Generally, I'd write this something like:

		netdev_warn(lp->ndev,
			    "Could not find clock ethernet controller property\n");

(without the period) which is 83 columns.

checkpatch makes exceptions for 80 column line
length maximums for format strings.

I've no real issue if you indent it back one.

fyi: this is 77 columns

		netdev_warn(lp->ndev,
			    "No clock ethernet controller property found\n");

About the message itself.

You dropped the "axienet_mdio_setup" function name.

I believe the dmesg output will look something like:

xilinx_temac 0000:01:00.0 (unregistered net_device): Could not find clock ethernet controller property.
xilinx_temac 0000:01:00.0 (unregistered net_device): Setting MDIO clock divisor to default 29

Because these 2 messages are effectively linked,
my preference would be to emit them on a single line,

Something like:

xilinx_temac 0000:01:00.0 (unregistered net_device): of_get_property("clock-frequency") not found - setting MDIO clock divisor to default 29

or

		netdev_warn(lp->ndev,
			    "of_get_property(\"clock-frequency\") not found - setting MDIO clock divisor to default %u\n",
			    DEFAULT_CLOCK_DIVISOR);

--
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/