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

Doug Ledford
2007-02-16T21:45:35Z

[-- Attachment #1: Type: text/plain, Size: 1999 bytes --]

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@redhat.com>
GPG KeyID: CFBFF194
http://people.redhat.com/dledford

Infiniband specific RPMs available at
http://people.redhat.com/dledford/Infiniband

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]