Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 1 Dec 2011 10:43:18 -0500 (EST)
From:      Rick Macklem <rmacklem@uoguelph.ca>
To:        John Baldwin <jhb@freebsd.org>
Cc:        Zack Kirsch <zack@freebsd.org>, mdf@freebsd.org, David Schultz <das@freebsd.org>, freebsd-arch@freebsd.org
Subject:   Re: Use of bool / stdbool.h in kernel
Message-ID:  <685533129.733983.1322754198902.JavaMail.root@erie.cs.uoguelph.ca>
In-Reply-To: <201112011012.16891.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
John Baldwin wrote:
> On Thursday, December 01, 2011 9:07:18 am Rick Macklem wrote:
> > David Schultz wrote:
> > > On Wed, Nov 30, 2011, John Baldwin wrote:
> > > > On Wednesday, November 30, 2011 12:13:53 am Bruce Evans wrote:
> > > > > On Tue, 29 Nov 2011 mdf@freebsd.org wrote:
> > > > >
> > > > > > At $WORK we have a hack in one of the *.mk files to allow
> > > > > > including
> > > > > > stdbool.h in the kernel and we use it extensively. This is
> > > > > > not
> > > > > > allowed by style(9), as far as I can tell, because the file
> > > > > > is
> > > > > > in
> > > > > > include/stdbool.h and those files are not allowed to be
> > > > > > included
> > > > > > in
> > > > > > kernel sources.
> > > > >
> > > > > Including stdbool.h in the kernel is not a style bug, but
> > > > > unsupported.
> > > > >
> > > > > > What I want to check on is, would it be acceptable to move
> > > > > > stdbool.h
> > > > > > from include/stdbool.h to sys/sys/stdbool.h (i.e. like
> > > > > > errno.h)
> > > > > > and
> > > > > > then include it in the kernel as <sys/stdbool.h>? That is,
> > > > > > is
> > > > > > the
> > > > >
> > > > > Would be a larger style bug, especially if it were actually
> > > > > used.
> > > > > Even its spellings of TRUE and FALSE are strange. Even in
> > > > > userland
> > > > > stdbool.h is considered so useful that it is never used in
> > > > > src/bin
> > > > > and is only used a few times on other src/*bin. src/bin never
> > > > > uses
> > > > > TRUE of FALSE either.
> > > >
> > > > I suspect there is some bias here though due to the fact that
> > > > there
> > > > wasn't
> > > > a standard bool type when most of this code was written. :) I
> > > > don't
> > > > think
> > > > that means we have to forgo use of the new type now that it is
> > > > in
> > > > fact
> > > > standardized in C99. I would be happy to have 'bool' available
> > > > and
> > > > the
> > > > lowercase 'true' and 'false' are fine with me.
> > >
> > > The lowercase 'true' and 'false' are intended to mimic C++, where
> > > they are keywords. Regardless of how you prefer to capitalize
> > > them, using them instead of 0 and 1 makes the intent much clearer.
> > > This is especially true in the kernel, where non-zero could mean
> > > true, or it could be an error code.
> > >
> > > Unfortunately, the "new type" is mostly useless, aside from
> > > improving readability. Unlike modern languages, C doesn't
> > > consider it a compile-time error to mix up bools and ints.
> > >
> > If this is added, would the style gods approve of the following:
> >
> > (A) bool test_func();
> >
> >     if (test_func())
> >            ...
> >
> > instead of:
> >
> > (B) int test_func();
> >
> >     if (test_func() != 0)
> >            ...
> >
> > Personally, I prefer the former, but understand that it isn't
> > currently style(9) compliant. Being able to do (A) instead of
> > (B) would be why I'd like stdbool.h to be added to the kernel,
> > if it will be allowed after the change?
> 
> My understanding is that (A) is fine if test_func() is returning an
> actual boolean value (so the return value is only true and false).
> A case where this isn't really true is when the function returns an
> errno as that is returning an int with many values other than just
> 0 and 1. Granted, it is a common style violation (I am guilty) to do:
> 
> if (error)
>     return (error);
> 
Yes, guilty as well. Once I became aware that it was a style(9) violation
I've resisted doing it, but barely;-)

Thanks for the commments, rick

> I also consider it a boolean test to test a single-bit flag in a flags
> field:
> 
> if (p->p_flag & P_PROFIL)
> 
> vs.
> 
> if ((p->p_flag & P_PROFIL) != 0)
> 
> If you had a multi-bit field though then I think it is appropriate to
> be more
> explicit about which values of that field are being tested for. Thus:
> 
> #define FLAGS_SIMPLE 0x00001
> #define FLAGS_FIELD 0x000fe
> 
> if (foo & FLAGS_SIMPLE)
> ...
> 
> if ((foo & FLAGS_FIELD) != 0)
> ...
> 
> --
> John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?685533129.733983.1322754198902.JavaMail.root>