Re: [IDE] meaningless #ifndef?

[Hugh Dickins]

On Tue, 20 Feb 2001, Vojtech Pavlik wrote:
> On Tue, Feb 20, 2001 at 05:45:52PM +0000, Hugh Dickins wrote:
> > > > byte eighty_ninty_three (ide_drive_t *drive)
> > > > {
> > > >         return ((byte) ((HWIF(drive)->udma_four) &&
> > > > #ifndef CONFIG_IDEDMA_IVB
> > > >                         (drive->id->hw_config & 0x4000) &&
> > > > #endif /* CONFIG_IDEDMA_IVB */
> > > >                         (drive->id->hw_config & 0x6000)) ? 1 : 0);
> > > > }
> 
> Well, the code looks weird. However, it doesn't behave the same when
> CONFIG_IDEDMA_IVB is enabled or not. If it is not, normal case, it's
> just:
> 
> (drive->id->hw_config & 0x6000)
> 
> If CONFIG_IDEDMA_IVB is enabled, it boils down to:
> 
> (drive->id->hw_config & 0x4000)
> 
> because the second bit test includes the earlier test already only
> loosening it. Because of that, it's superfluous. And the code relies on
> the compiler to optimize it out.

Thank you for re-explaining this to me.
Yes, you are right, I was wrong, I now
admit my &s and &&s behave like yours,
and I apologize to Andre!

> If written like:
> 
> #ifndef CONFIG_IDEDMA_IVB
> #define IDE_UDMA_MASK 0x4000
> #else
> #define IDE_UDMA_MASK 0x6000
> #endif /* CONFIG_IDEDMA_IVB */
> 
> byte eighty_ninty_three (ide_drive_t *drive)
> {       
>         return HWIF(drive)->udma_four &&
> 		(drive->id->hw_config & IDE_UDMA_MASK);
> }
> 
> it'd be probably somewhat clearer.

That would certainly have helped us!

Hugh