Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 03 May 2016 11:19:44 -0700
From:      John Baldwin <jhb@freebsd.org>
To:        Bruce Evans <brde@optusnet.com.au>
Cc:        Pedro Giffuni <pfg@freebsd.org>, "Ngie Cooper (yaneurabeya)" <yaneurabeya@gmail.com>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r298933 - in head: share/man/man9 sys/amd64/include sys/dev/acpica sys/dev/drm2 sys/dev/drm2/i915 sys/kern sys/sys sys/x86/acpica sys/x86/x86
Message-ID:  <1928389.rOu33C1eaq@ralph.baldwin.cx>
In-Reply-To: <20160504031930.A3395@besplex.bde.org>
References:  <201605021800.u42I0cjK084243@repo.freebsd.org> <8989101.JIAk4LJusf@ralph.baldwin.cx> <20160504031930.A3395@besplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday, May 04, 2016 03:58:40 AM Bruce Evans wrote:
> On Tue, 3 May 2016, John Baldwin wrote:
> 
> > On Tuesday, May 03, 2016 03:52:56 PM Bruce Evans wrote:
> >> On Mon, 2 May 2016, Pedro Giffuni wrote:
> >...
> >>> TBH, I thought so too, but I avoided applying such changes to headers,
> >>> and I haven't touched _bitset.h,
> >>
> >> _foo.h headers cannot use howmany() due to namespace pollution.
> >>
> >> _bitset.h was already broken, unless it is supposed to be kernel-only --
> >> it uses howmany().  It is kernel-only according to its documention --
> >> bitset is only documented in kernel manpages (in a single unreadable one
> >> than is linked ad nauseum).
> >
> > cpuset.h is used in userland for cpuset_getaffinity(2), etc.
> >
> >> It is otherwise fairly clean.  It defines the symbols BITSET_DEFINE,
> >> BITSET_T_INITIALIZER, and BITSET_SET in the application namespace
> >> This is not completely clean for a _foo.h header.  All other BITSET*
> >> macros are already in bitset.h  I think only BITSET_DEFINE should be
> >> in _bitset.h (for use declarations in other headers).
> >>
> >> select.h avoids this problem by defining its own howmany() macro.  This
> >> seems to be correct except for the bogus ifdef around the private macro.
> >> This ifdef is a little more than a style bug (verboseness) -- it breaks
> >> detection of other definitions that might be different.  Lexical
> >> differences wouldn't matter, but it is easier to never have them.
> >>
> >> Old versions of select polluted <sys.types.h>.  The select macros just
> >> used howmany().  howmany() was in <sys/types.h> too.
> >
> > I would be happy to fix _bitset.h and _cpuset.h to not need sys/param.h.
> > However, they also use NBBY which is defined in sys/param.h.  _sigset.h
> > gets around this because it uses an array of uint32_t and hardcodes a
> > shift count of 5 in _SIG_WORD() and a mask of 31 in _SIG_BIT().  If you
> > think it is fine to hardcode '8' instead of 'NBBY' I'll do that.  Hmm,
> > sys/select.h hardcodes '8' for _NFDBITS, so I guess that is fine.
> 
> NBBY can be cleaned up too.  I rather like it, but it is bogus in C90
> since it is spelled CHAR_BIT there, and it is now more bogus in POSIX
> since POSIX started specifying 8-bit bytes in 2001.  Thus 8 is the
> correct spelling of it in the implementation where you don't want to
> expose a macro that makes it clearer what this magic 8 is.

Ok.

> BTW, I don't like select's and bitset's use of longs.  Using unsigned
> for select is a historical mistake.  Bitset apparently copied select
> except it unimproved to signed long.  Bitstring uses unsigned char with
> no optimizations.  Sigset uses uint32_t with no obvious optimizations,
> but compilers do a good job with with it due to its fixed size.  I doubt
> that the manual optimization of using a wider size is important.

I agree, but cpuset_t is already part of the ABI in existing releases. :(
Changing it to uint32_t would break the ABI for big-endian platforms.

-- 
John Baldwin



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