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

[Doug Ledford]

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

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

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