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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 16 Feb 2007 16:43:04 -0500
From:	Doug Ledford <dledford@...hat.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	James Bottomley <James.Bottomley@...elEye.com>,
	ricknu-0@...dent.ltu.se, linux-kernel@...r.kernel.org,
	linux-scsi@...r.kernel.org
Subject: Re: [PATCH] drivers/scsi/aic7xxx_old: Convert to generic
	boolean-values

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

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

Download attachment "signature.asc" of type "application/pgp-signature" (190 bytes)