Re: [PATCH] drivers/scsi/aic7xxx_old: Convert to genericboolean-values

From: Doug Ledford
Date: Fri Feb 16 2007 - 16:46:03 EST

On Fri, 2007-02-16 at 10:50 -0800, Andrew Morton wrote:

> Me no understand.
> If you take the specific example of
> void
> ahd_set_syncrate(struct ahd_softc *ahd, struct ahd_devinfo *devinfo,
> u_int period, u_int offset, u_int ppr_options,
> u_int type, int paused)
> then if is crufty, inappropriate and wrong that `paused' is a scalar type.

Although you picked a code segment out of the modern aic7xxx, there is a
matching similar one in aic7xxx_old. Now, in all fairness, I was at one
point playing with a much more preemptable model for that driver that
allowed nested pauses, at which point the value of pause would have made
sense to be scalar, but that was a *long* time ago.

> It's just not true or sensible that the code is written so that `paused'
> can take a value of seventy eight. It _is_ a boolean. It is a truth
> value. Declaring it as such in the source is all goodness. Passing the
> value `true' into calls to this function improve readability over passing
> "1".

Hence the reason for the original upper case TRUE/FALSE. I have to
admit, I don't really like the lower case true/false, it looks like a
variable that can be assigned, thereby changing the implementation of
the function call when in fact each calling location is hard coding a
constant. But, that's just me and my crufty old C that differentiates
between hard coded things and variables via case.

> So I don't agree with (or understand) your objections. But I can certainly
> understand reluctance to merge a large-but-minor, do-nothing-much patch into
> a large and not-very-maintained driver.

Hehehe...and here I was thinking of factoring that thing into files and
actually bringing it into the current century.

Doug Ledford <dledford@xxxxxxxxxx>

Infiniband specific RPMs available at

Attachment: signature.asc
Description: This is a digitally signed message part