All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.