netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Németh Márton" <nm127@freemail.hu>
To: Stefan Richter <stefanr@s5r6.in-berlin.de>,
	David Newall <davidn@davidnewall.com>
Cc: Jeff Garzik <jgarzik@pobox.com>,
	netdev@vger.kernel.org, Trivial Patch Monkey <trivial@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] 8139too: clean up spaces and TABs
Date: Sun, 15 Jun 2008 09:50:23 +0200	[thread overview]
Message-ID: <4854C9BF.8040900@freemail.hu> (raw)
In-Reply-To: <485421C1.8010102@s5r6.in-berlin.de>

Stefan Richter wrote:
> Németh Márton wrote:
>> From: Márton Németh <nm127@freemail.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


  parent reply	other threads:[~2008-06-15  7:50 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-14 12:36 [PATCH 1/2] 8139too: clean up spaces and TABs Németh Márton
2008-06-14 13:33 ` Stefan Richter
2008-06-14 18:43   ` Németh Márton
2008-06-14 19:53     ` Stefan Richter
2008-06-14 21:21       ` Ondrej Zary
2008-06-14 21:39         ` Francois Romieu
2008-06-15  7:50       ` Németh Márton [this message]
2008-06-15  7:52         ` [PATCH] 8139too: some style cleanups Németh Márton
2008-06-27  6:09           ` Jeff Garzik
2008-06-16 15:07         ` [PATCH 1/2] 8139too: clean up spaces and TABs Stefan Richter
2008-06-16 16:31           ` Stephen Hemminger
2008-06-15  6:21     ` David Newall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4854C9BF.8040900@freemail.hu \
    --to=nm127@freemail.hu \
    --cc=davidn@davidnewall.com \
    --cc=jgarzik@pobox.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=stefanr@s5r6.in-berlin.de \
    --cc=trivial@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).