Re: [IDE] meaningless #ifndef?

Hugh Dickins (hugh@veritas.com)
Tue, 20 Feb 2001 19:08:37 +0000 (GMT)


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

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