Re: [PATCH 01/10] ide: add flags query macros

[Borislav Petkov]

Hi,

On Sun, Feb 15, 2009 at 02:35:12PM +0100, Sam Ravnborg wrote:
> On Sun, Feb 15, 2009 at 01:08:03PM +0100, Borislav Petkov wrote:
> > There should be no functionality change resulting from this patch.
> > 
> > Signed-off-by: Borislav Petkov <petkovbb@xxxxxxxxx>
> > ---
> >  include/linux/ide.h |  166 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 166 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/linux/ide.h b/include/linux/ide.h
> > index c75631c..f133062 100644
> > --- a/include/linux/ide.h
> > +++ b/include/linux/ide.h
> > @@ -497,6 +497,82 @@ enum {
> >  	IDE_AFLAG_NO_AUTOCLOSE		= (1 << 24),
> >  };
> >  
> > +#define ide_drv_drq_int(drive) \
> > +	((drive)->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT)
> 
> Why not use a static inline here so we get proper typecheck.
> And then convert the return result to a bool (0/1) so you
> do not have to do this at the call site.
> 
> I counted at least three places in ide-cd that does a local
> transformation to a bool and I saw nowhere the actual bit value
> was used.

I'm assuming you're talking about those places (and similar):

drive->dma = !!(drive->dev_flags & IDE_DFLAG_USING_DMA);

Well, actually we almost never use the 0/1 bool value and this one
case is more of an exception. If you take a closer look, we don't have
setters/getters ..., well let's call them methods as it is in the
OO-world, and we simply access the bare flags. So the macros as such are
simply to save some stack and improve readability and since the whole
thing keeps changing we might just as well turn them into static inlines
one fine day :).

Bart, what do you think?

-- 
Regards/Gruss,
    Boris.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/