linux-kernel - Re: [PATCH 02/39] arch/alpha: Update WARN uses

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 31 Oct 2010 14:11:00 +1300
From:	Michael Cree <mcree@...on.net.nz>
To:	Joe Perches <joe@...ches.com>
CC:	Matt Turner <mattst88@...il.com>, Jiri Kosina <trivial@...nel.org>,
	Richard Henderson <rth@...ddle.net>,
	Ivan Kokshaysky <ink@...assic.park.msu.ru>,
	linux-alpha@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 02/39] arch/alpha: Update WARN uses

On 31/10/10 10:28, Joe Perches wrote:
> On Sat, 2010-10-30 at 17:17 -0400, Matt Turner wrote:
>> On Sat, Oct 30, 2010 at 5:08 PM, Joe Perches<joe@...ches.com>  wrote:
>>> Coalesce long formats.
>>> Align arguments.
>>> Signed-off-by: Joe Perches<joe@...ches.com>
>>>   arch/alpha/kernel/pci-sysfs.c |   14 ++++++--------
>>>   1 files changed, 6 insertions(+), 8 deletions(-)
>>> diff --git a/arch/alpha/kernel/pci-sysfs.c b/arch/alpha/kernel/pci-sysfs.c
>>> @@ -44,10 +44,9 @@ static int __pci_mmap_fits(struct pci_dev *pdev, int num,
>>> -       WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on %s BAR %d "
>>> -               "(size 0x%08lx)\n",
>>> -               current->comm, sparse ? " sparse" : "", start, start + nr,
>>> -               pci_name(pdev), num, size);
>>> +       WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on %s BAR %d (size 0x%08lx)\n",
>>> +            current->comm, sparse ? " sparse" : "", start, start + nr,
>>> +            pci_name(pdev), num, size);
>>> @@ -261,10 +260,9 @@ static int __legacy_mmap_fits(struct pci_controller *hose,
>>> -       WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on hose %d "
>>> -               "(size 0x%08lx)\n",
>>> -               current->comm, sparse ? " sparse" : "", start, start + nr,
>>> -               hose->index, size);
>>> +       WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on hose %d (size 0x%08lx)\n",
>>> +            current->comm, sparse ? " sparse" : "", start, start + nr,
>>> +            hose->index, size);
>> What's accomplished here? It looks like you joined two strings (which
>> were separated as to not overflow the 80-char limit) and spaced a
>> couple lines over.
>>
>> I don't see how this makes anything clearer.
>
> This one is just a whitespace cleanup.
> Ignore it or nack it if you want.

I prefer the original indenting as it clearly stands the `1' on its own 
thus emphasising that the warning will be printed everytime the WARN() 
statement is executed.  I surmise that the original programmer thought 
it important to emphasise this as it might not be considered the 
"normal" usage of WARN().

I agree that, in general, coalesced strings aid grepping but this broken 
string isn't one that will benefit.

I recall the story that when someone pointed out parallel fifths (an 
absolute no-no according to the rules of classical harmony) in a 
composition of Beethoven's to Beethoven himself, Beethoven was so 
indignant that he proceeded to compose a set of variations on a theme 
where in each variation he extensively violated one of the rules of 
classical harmony, and wrote down the bottom of each variation "You Ass" 
to the authority who had made the rule.

The moral here is that when a _competent_ programmer breaks the "style 
guide" they are most likely doing so for a very good reason.  Thus, 
those who would submit whitespace fix patches should take time to ask 
themselves why the original programmer has laid out the code in such a 
fashion and learn from it, before submitting so-called "fixes".

Cheers
Michael.
--
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/