linux-kernel - Re: [PATCH 19/32] drivers/hwmon/pc87360.c: Use pr_fmt and pr

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:	Tue, 09 Nov 2010 01:59:44 -0800
From:	Joe Perches <joe@...ches.com>
To:	Jean Delvare <khali@...ux-fr.org>
Cc:	Guenter Roeck <guenter.roeck@...csson.com>,
	Jim Cromie <jim.cromie@...il.com>, lm-sensors@...sensors.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 19/32] drivers/hwmon/pc87360.c: Use pr_fmt and
 pr_<level>

On Tue, 2010-11-09 at 10:31 +0100, Jean Delvare wrote:
> On Tue, 19 Oct 2010 23:51:43 -0700, Joe Perches wrote:
> > Added #define pr_fmt KBUILD_MODNAME ": " fmt
> > Converted printks to pr_<level>
> > Coalesced any long formats
> > Removed prefixes from formats
> > 
> > Signed-off-by: Joe Perches <joe@...ches.com>
> > ---
> >  drivers/hwmon/pc87360.c |   32 +++++++++++++-------------------
> >  1 files changed, 13 insertions(+), 19 deletions(-)
> The following is left in the driver:
> #ifdef DEBUG
> 			printk(KERN_DEBUG "pc87360: Fan 1: mon=%d "
> 			       "ctrl=%d inv=%d\n", (confreg[0]>>2)&1,
> 			       (confreg[0]>>3)&1, (confreg[0]>>4)&1);
> 			printk(KERN_DEBUG "pc87360: Fan 2: mon=%d "
> 			       "ctrl=%d inv=%d\n", (confreg[0]>>5)&1,
> 			       (confreg[0]>>6)&1, (confreg[0]>>7)&1);
> 			printk(KERN_DEBUG "pc87360: Fan 3: mon=%d "
> 			       "ctrl=%d inv=%d\n", confreg[1]&1,
> 			       (confreg[1]>>1)&1, (confreg[1]>>2)&1);
> #endif
> 
> Is there any reason why you didn't convert these to pr_debug()?

No.  These should be converted to pr_debug as well.

The conversion was done via script and the script doesn't
convert printk(KERN_DEBUG to pr_debug to avoid changing output.

Let me know if you want another patch.

> > @@ -1071,14 +1072,11 @@ static int __init pc87360_find(int sioaddr, u8 *devid, unsigned short *addresses
> >  				confreg[3] = superio_inb(sioaddr, 0x25);
> >  
> >  				if (confreg[2] & 0x40) {
> > -					printk(KERN_INFO "pc87360: Using "
> > -					       "thermistors for temperature "
> > -					       "monitoring\n");
> > +					pr_info("Using thermistors for temperature monitoring\n");
> 
> I know checkpatch.pl no longer complains about long lines when they
> include a string, but 98 columns seems excessive to me. It would be
> easy enough to spread over two lines.

Also converted via a script.

If the string is split, it doesn't matter where it's split.
The split just makes it harder to use grep.

Personally, I think it's better to ignore the line length
of format strings completely.  Once it wraps in an editor,
it really doesn't matter how long the line is.

Of course, there are multiple strings in this file that
use dev_<level> that are split across lines that could be
converted as well so it's up to you if you want to leave
those as is.  Let me know if you want another patch to
integrate those dev_<level> format strings.

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