All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robert Hancock <hancockr@shaw.ca>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-kernel <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Jesse Barnes <jbarnes@virtuousgeek.org>
Subject: Re: [PATCH -mm] 1/2: MMCONFIG: validate against ACPI motherboard resources
Date: Wed, 30 May 2007 08:35:13 -0600	[thread overview]
Message-ID: <465D8BA1.1080800@shaw.ca> (raw)
In-Reply-To: <alpine.LFD.0.98.0705292125511.26602@woody.linux-foundation.org>

Linus Torvalds wrote:
> On Tue, 29 May 2007, Robert Hancock wrote:
>> This path adds validation of the MMCONFIG table against the ACPI reserved
>> motherboard resources.
> 
> Please fix the formatting of your code.
> 
> "for" and "if" are not functions, and they have a space before the 
> parenthesis.
> 
> And pretty much every single conditional in this patch is spread out over 
> two or more lines and has at least three different indentations. There's 
> something wrong here. Code can't look this bad and still be fine. Some of 
> this looks like random whitespace noise:
> 
> +               if(is_acpi_reserved(cfg->address,
> +                                   cfg->address + size - 1))
> +                       printk(KERN_NOTICE "PCI: MCFG area at %Lx reserved "
> +                               "in ACPI motherboard resources\n",
> +                               cfg->address);
> +               else {
> 
> That's just horrid. Please try to make the code _look_ nicer.

I'll try and fix up the formatting and repost this patch. I suspect some
of the issues are from the added code clashing with the way the existing
code was formatted.

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/


  parent reply	other threads:[~2007-05-30 14:35 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-30  4:03 [PATCH -mm] 1/2: MMCONFIG: validate against ACPI motherboard resources Robert Hancock
2007-05-30  4:45 ` Linus Torvalds
2007-05-30  8:41   ` Matt Keenan
2007-05-30 14:35   ` Robert Hancock [this message]
2007-05-30 15:05     ` Linus Torvalds
2007-05-30 15:38       ` Mark Lord
2007-05-30 23:33         ` Robert Hancock
2007-05-30 17:23       ` Dave Jones
2007-05-30 17:37         ` Linus Torvalds
2007-05-30 17:54           ` Dave Jones
2007-05-30 17:40         ` Andrew Morton
2007-05-31  1:02       ` Robert Hancock
2007-06-04 12:12 ` Olivier Galibert
2007-06-04 14:35   ` Robert Hancock
2007-06-04 15:37   ` Jesse Barnes

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=465D8BA1.1080800@shaw.ca \
    --to=hancockr@shaw.ca \
    --cc=akpm@linux-foundation.org \
    --cc=jbarnes@virtuousgeek.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.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.