linux-kernel - Re: [PATCH v3] Add support for the Philips SA56004 temperature sensor.

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:	Sun, 26 Jun 2011 13:20:00 -0700
From:	Guenter Roeck <guenter.roeck@...csson.com>
To:	Fubo Chen <fubo.chen@...il.com>
CC:	"sdevrien@...co.com" <sdevrien@...co.com>,
	"khali@...ux-fr.org" <khali@...ux-fr.org>,
	"lm-sensors@...sensors.org" <lm-sensors@...sensors.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] Add support for the Philips SA56004 temperature
 sensor.

On Sun, Jun 26, 2011 at 03:54:37PM -0400, Fubo Chen wrote:
> On Mon, Jun 6, 2011 at 12:40 PM,  <sdevrien@...co.com> wrote:
> > +       } else
> > +       if (man_id == 0xA1) { /*  NXP Semiconductor/Philips */
> 
> No kernel coder writes like this. Has this ever been run through checkpatch ?
> 
What a nice comment. Quite obviously _you_ did not run the patch through checkpatch
to back up yout claim. I did:

total: 0 errors, 0 warnings, 151 lines checked

The code format is in line with other if/else constructs in the same function,
which coincidentially also pass through checkpatch. Which you did not bother
checking either (neither the format of the code in lm90.c nor the results
of checkpatch over it).

Before making statements like this, you might want to consider spending some time
with the actual code - amongst others to see if the new code fits into the structure
of existing code.

If you want to improve code formatting in this or in other files, I would suggest 
to submit patches.

Thanks,
Guenter
--
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/