linux-kernel - Re: [PATCH 1/2] 8139too: clean up spaces and TABs

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, 15 Jun 2008 09:50:23 +0200
From:	Németh Márton <nm127@...email.hu>
To:	Stefan Richter <stefanr@...6.in-berlin.de>,
	David Newall <davidn@...idnewall.com>
CC:	Jeff Garzik <jgarzik@...ox.com>, netdev@...r.kernel.org,
	Trivial Patch Monkey <trivial@...nel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] 8139too: clean up spaces and TABs

Stefan Richter wrote:
> Németh Márton wrote:
>> From: Márton Németh <nm127@...email.hu>
>>
>> Clean up the following errors and warnings reported by checkpatch.pl:
> 
> Is 8139too in active development or are there people actively fixing 
> current bugs in it?  If not, a whitespace cleanup may be considered a 
> waste of time.  There are even a few valid arguments that such changes 
> are harmful then.
> 
>>  - space prohibited between function name and open parenthesis '('
>>  - space required before the open brace '{'
>>  - code indent should use tabs where possible
>>  - line over 80 characters
>>  - spaces required around that '=' (ctx:VxW)
> 
> Did you check that your whitespace changes are indeed only whitespace 
> changes, i.e. that resulting assembler is unchanged?  If you checked it, 
> it's worth mentioning in the submission.

Yes, I checked this. When I compared the "objdump -d" output I found that
only the function calls are changed where the __LINE__ is a parameter.

Sorry about taking your time. My intention was to make the 8139too source
code better as I am using this driver actively to communicate with the
digital world. I have chosen the checkpatch.pl's output to compare the
whether the old and the new code are better or not. Maybe that was a mistake.

I would like to try it again, but in a different way. I collected here
the different problems reported by checkpatch.pl. I divided it to two
groups according to your comments which I think it worth to deal with:

 + ERROR: Macros with complex values should be enclosed in parenthesis
 + WARNING: __func__ should be used instead of gcc specific __FUNCTION__
 + WARNING: plain inline is preferred over __inline__
 + WARNING: Use #include <linux/io.h> instead of <asm/io.h>
 + WARNING: Use #include <linux/uaccess.h> instead of <asm/uaccess.h>

and the second group which it seems that may hurt somebody:

 - ERROR: code indent should use tabs where possible
 - ERROR: space required after that ',' (ctx:VxV)
 - ERROR: space required before the open brace '{'
 - ERROR: space required before the open parenthesis '('
 - ERROR: spaces required around that '=' (ctx:VxW)
 - ERROR: trailing statements should be on next line
 - WARNING: braces {} are not necessary for single statement blocks
 - WARNING: do not add new typedefs
 - WARNING: line over 80 characters
 - WARNING: space prohibited between function name and open parenthesis '('

This is a much smaller set of changes. What do you think?

Regards,

	Márton Németh

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