RE: [PATCH 1/4] DSPBRIDGE: Fix macros that break when inside an if/else

[Phil Carmody <ext-phil.2.carmody@nokia.com>]

On Tue, 2009-07-14 at 13:05 +0200, ext Menon, Nishanth wrote:
> Phil,
> > -----Original Message-----
> > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> > owner@vger.kernel.org] On Behalf Of Phil Carmody
> > Sent: Tuesday, July 14, 2009 6:03 AM
> > 
> > On Fri, 2009-07-10 at 01:51 +0200, ext Guzman Lugo, Fernando wrote:
> > > Thanks for the patch, it only has indentation problems, this is the
> > checkpatch output:
> > >
> > > WARNING: suspect code indent for conditional statements (8, 12)
> > > #34: FILE: drivers/dsp/bridge/pmgr/wcd.c:152:
> > > +       if (DSP_SUCCEEDED(status)) {                    \
> > > +           if (unlikely((src) == NULL) ||              \
> > >
> > > WARNING: line over 80 characters
> > > #36: FILE: drivers/dsp/bridge/pmgr/wcd.c:154:
> > > +               unlikely(copy_from_user(dest, src, (elements) *
> > sizeof(*(dest))))) { \
> > >
> > > WARNING: suspect code indent for conditional statements (8, 12)
> > > #46: FILE: drivers/dsp/bridge/pmgr/wcd.c:164:
> > > +       if (DSP_SUCCEEDED(status)) {                    \
> > > +           if (unlikely((dest) == NULL) ||
> > \
> > >
> > > WARNING: line over 80 characters
> > > #48: FILE: drivers/dsp/bridge/pmgr/wcd.c:166:
> > > +               unlikely(copy_to_user(dest, src, (elements) *
> > sizeof(*(src))))) { \
> > >
> > > total: 0 errors, 4 warnings, 48 lines checked
> > >
> > > Could you please fix that warning please?
> > 
> > I suspect the only way to fix those warnings would either introduce
> > other warnings, or \
> Gentle query: Have we already tried this or is it just a suspicion?

Being an Englishman, I use various forms of irony more than others do;
that particular form's called "litotes". It indeed is not a suspicion -
I tried about 4 or 5 variations, none of which were both readable and
warning-free. In the end I decided that whatever was closest to the
original would be safest.

> > 							would \
> > 							lead \
> > 							to \
> > 							utterly \
> > 							unread- \
> > 							able \
> > 							code.
> > 
> > If you check how and why the original TI-originated version of the code
> > does not follow the linux coding standards, the difficulties we would
> > have making a warning-free patch of it should be apparent.
> 
> Past is past. The idea was not to introduce anymore warning code.

The TI code would trigger the following 4 warnings:
WARNING: suspect code indent for conditional statements (4, 12)
WARNING: line over 80 characters
WARNING: suspect code indent for conditional statements (4, 12)
WARNING: line over 80 characters
4 warnings isn't more than 4 warnings, it's no change - and as I mention
above, in the absence of an easy fix, that was deliberate.

Phil