From: James Bottomley <James.Bottomley@SteelEye.com>
To: Richard Knutsson <ricknu-0@student.ltu.se>
Cc: Andrew Morton <akpm@linux-foundation.org>,
linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org
Subject: Re: [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values
Date: Fri, 16 Feb 2007 12:23:34 -0600 [thread overview]
Message-ID: <1171650214.3443.41.camel@mulgrave.il.steeleye.com> (raw)
In-Reply-To: <45D5F21B.2010804@student.ltu.se>
On Fri, 2007-02-16 at 19:04 +0100, Richard Knutsson wrote:
> James Bottomley wrote:
> > On Mon, 2007-02-12 at 12:27 -0800, Andrew Morton wrote:
> >
> >> Given that we now have a standard kernel-wide, c99-friendly way of
> >> expressing true and false, I'd suggest that this decision can be revisited.
> >>
> >> Because a "true" is significantly more meaningful (and hence readable)
> >> thing than a bare "1".
> >>
> >
> > OK, I'm really not happy with doing this for three reasons:
> >
> > 1. It's inviting huge amounts of driver churn changing bitfields to
> > booleans
> >
> Have been some work done already. Has there been any problems?
There's always an issue when two people work on the same driver ... it
causes patch conflicts, which is why we try to avoid it where we can.
> > 2. I do find it to be a readability issue. Like most driver writers,
> > I'm used to register layouts, and those are simple bitfields, so I don't
> > tend to think true and false, I think 1 and 0.
> >
> It is a fundamental difference between an integer and a boolean. Have
> you seen anyone trying to do "bool var = true + true;"? ;)
I don't quite see how this is relevant to the readability issue?
> > 3. Having a different, special, type for single bit bitfields (while
> > still using u<n> for multi bit bitfields) is asking for confusion, and
> > hence trouble at the driver level.
> >
> I don't think a boolean should be view as a single bit bitfield. Ex:
> u8 a:1;
> ...
> int b = 4 + a;
> is obviously not a boolean, while:
> u8 a:1;
> ...
> if (a)
> is, and a should be "bool a:1;" (imho)
This again, doesn't really address the argument. I'm saying I'd rather
not have confusion over what types to use in the driver. You're saying
that if you only check the value for truth or falsehood it should be a
boolean. That's actually worse than I was anticipating because you're
now saying that single bit bitfields may or may not be booleans
depending on use. This looks like worse potential for confusion than
before.
James
next prev parent reply other threads:[~2007-02-16 18:23 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-02-10 17:46 [PATCH] drivers/scsi/aic7xxx_old: Convert to generic boolean-values Richard Knutsson
2007-02-10 18:27 ` James Bottomley
2007-02-10 20:35 ` Richard Knutsson
2007-02-10 20:43 ` Richard Knutsson
2007-02-12 20:27 ` Andrew Morton
2007-02-16 16:42 ` James Bottomley
2007-02-16 18:04 ` Richard Knutsson
2007-02-16 18:23 ` James Bottomley [this message]
2007-02-16 19:10 ` Richard Knutsson
2007-02-16 18:34 ` Andrew Morton
2007-02-16 18:42 ` James Bottomley
2007-02-16 18:50 ` Andrew Morton
2007-02-16 21:43 ` Doug Ledford
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1171650214.3443.41.camel@mulgrave.il.steeleye.com \
--to=james.bottomley@steeleye.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=ricknu-0@student.ltu.se \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox