linux-kernel - Re: [PATCH] staging: wavelan: fix coding style of first 1000 lines in wavelan.c

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: <1268067279.1925.41.camel@Joe-Laptop.home>
Date:	Mon, 08 Mar 2010 08:54:39 -0800
From:	Joe Perches <joe@...ches.com>
To:	Jason Dreisbach <jtdreisb@...il.com>
Cc:	jt@....hp.com, gregkh@...e.de--to, jpirko@...hat.com,
	linville@...driver.com, jtdreisb@...il.com--cc,
	linux-kernel@...r.kernel.org, linux-wireless@...r.kernel.org
Subject: Re: [PATCH] staging: wavelan: fix coding style of first 1000 lines
 in wavelan.c

On Mon, 2010-03-08 at 02:20 -0800, Jason Dreisbach wrote:
> From: Jason <jtdreisb@...il.com>
> diff --git a/drivers/staging/wavelan/wavelan.c b/drivers/staging/wavelan/wavelan.c
> index 54ca631..37ede43 100644
> --- a/drivers/staging/wavelan/wavelan.c
> +++ b/drivers/staging/wavelan/wavelan.c
[]
> @@ -279,7 +285,8 @@ static void update_psa_checksum(struct net_device * dev, unsigned long ioaddr, u
>  
>  	if (crc != 0)
>  		printk(KERN_WARNING
> -		       "%s: update_psa_checksum(): CRC does not agree with PSA data (even after recalculating)\n",
> +		       "%s: update_psa_checksum(): CRC does not \
> +		       agree with PSA data (even after recalculating)\n",
>  		       dev->name);
>  #endif				/* DEBUG_IOCTL_INFO */
>  #endif				/* SET_PSA_CRC */

Hi Jason.

What you've done here actually adds a defect.

Breaking up a string using line continuations adds the
initial spaces on the next line to the string.

Ignore the checkpatch error for long format lines.
They are not an error and are actually the preferred style.

One thing you could do is to change the logging form from:
	printk(KERN_<level> "%s: fmt...", dev->name, args);
to
	netdev_<level>(dev, "fmt...", args);

as this may now be a preferred style.
It gives a bit more information in the logs.

When dev is a struct device *, the conversion should be from:
	printk(KERN_<level> "%s: fmt...", dev->name, args);
to:
	dev_<level>(dev, "fmt...", args);

Another thing you could do would be to take the
embedded function name out of the format string
and use "%s() ...", __func__ as this can save
some text space when there's more than 1 use of
the function name and if the function is renamed
or refactored, the right function name is used.

So this gets converted from:
		printk(KERN_WARNING
		       "%s: update_psa_checksum(): CRC does not agree with PSA data (even after recalculating)\n",
		       dev->name);
to:
		netdev_warn(dev, "%s(): CRC does not agree with PSA data (even after recalculating)\n",
			    __func__);


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