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

[Michael Cree]

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@xxxxxxxxxxx>  wrote:
> > > Coalesce long formats.
> > > Align arguments.
> > > Signed-off-by: Joe Perches<joe@xxxxxxxxxxx>
> > >   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-alpha" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html