Re: ide cleanup

From: Bill Davidsen (davidsen@tmr.com)
Date: Mon Feb 11 2002 - 14:15:59 EST


On Sat, 9 Feb 2002, Vojtech Pavlik wrote:

> On Sat, Feb 09, 2002 at 01:19:58PM -0500, Bill Davidsen wrote:
> > On Wed, 6 Feb 2002, Pavel Machek wrote:
> >
> > > -#ifdef CONFIG_BLK_DEV_PDC4030
> > > if (IS_PDC4030_DRIVE) {
> > > extern ide_startstop_t promise_rw_disk(ide_drive_t *, struct request *, unsigned long);
> > > return promise_rw_disk(drive, rq, block);
> > > }
> > > -#endif /* CONFIG_BLK_DEV_PDC4030 */
> >
> > Am I reading this totally wrong, or do you really think it's a good idea
> > to test for a drive even if the user didn't configure such hardware?
>
> Well, since IS_PDC4030_DRIVE will always be 0 in that case, the test
> will be optimized out ...

That's currently true, but since there are other places in the kernel
which use the ifdef, it's unobvious to read because you have to know that
the drive type test macro definition has side effects, it depends on a
behaviour of gcc which may not always be true, and it saves nothing, just
moves the code from the preprocessor to the optimizer, I don't see
removing it in general.

Just my opinion, code should be written to be EASILY read rather than as
if to be entered in an obfuscated C contest. This is not that bad, but the
test always being false is not obvious without a study of the entire use
of the macro, the ifdef is.

-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

- 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/



This archive was generated by hypermail 2b29 : Fri Feb 15 2002 - 21:00:41 EST