svn commit: r249355 - head/lib/libkvm

[Bruce Evans]

On Thu, 11 Apr 2013, Gleb Smirnoff wrote:

>  Juli,
>
> On Thu, Apr 11, 2013 at 11:05:28AM -0700, Juli Mallett wrote:
> J> > On Thu, Apr 11, 2013 at 09:07:25PM +1000, Bruce Evans wrote:
> J> > B> Just routine avoidance of namespace pollution.  This is easy in such a
> J> > B> simple header.
> J> >
> J> > Sorry, with all respect, but I can't call including sys/types.h
> J> > a namespace pollution.
> J> >
> J> > Ok, even you force me to name it that way, still I would prefer
> J> > namespace pollution instead of handmade copy pasted typedefs.
> J>
> J> But Gleb, making such changes unilaterally is a bit of a leap.  The
> J> project has mostly accepted Bruce's wisdom about trying to minimize
> J> and reduce namespace pollution.  Now, this isn't a standard header so
> J> it's quite a bit less of a concern, but it's not no concern.  If you
> J> think that we should reverse our trend on including
> J> namespace-polluting headers in system headers, we should discuss that
> J> on arch@, and it shouldn't be something that's done without any
> J> discussion or consideration.
> J>
> J> Should we expect further changes of this nature (and of the proposed
> J> nature removing __size_t and __ssize_t use) if you make changes to
> J> other headers as part of your work?  Are you going to add
> J> <sys/types.h> to every header currently using <sys/_types.h> in a
> J> single go, or will you be doing that a little at a time when making
> J> functional changes?
>
> Your suggestion? Typedef standard uint64_t manually as size_t and ssize_t
> already are done? Can you please define amount of standard types needed
> for kvm.h (or any abstract header) that would give a permission to include
> sys/types.h instead of typedefing all these types via a cut-n-paste
> surrounded by ifdefs?

A number larger than any header needs.

<unistd.h> has 7 ifdefs for types (as part of carefully avoiding including
sys/types.h>) and 13 _FOO_DECLARED ifdefs for other things (maily for
functions).  <netinet/in.h> has 8 for types.  <stdio.h> has 10 for functions.
These are the largest users of _FOO_DECLARED in the installed /usr/include.

> I am all against namespace pollution, but not when it comes to sys/types.h.

It has lots of historical BSD pollution (mainly endian and select).
This is carefully ifdefed under _BSD_VISIBLE.  This is another reason
not to depend on sys/types.h declaring uint64_t or anything else not
specifed by POSIX in any other header -- someone might define
_BSD_VISIBLE as 0.  It happens that uint64_t is not under any ifdef.
This is justified since it is a type ending with the reserved suffix
_t.  kvm.h is a BSD header, so defining _BSD_VISIBLE as 0 before
including it is probably invalid.  POSIX headers need to be more
careful.

The tinderbox failure mail showed another namespace error -- a new prototype
in kvm.h uses u_long.  The declaration of this in sys/types.h would _is_
turned off by BSD_VISIBLE = 0.  Old prototypes in kvm.h avoid this problem
by spelling unsigned long as itself.  Using the verbose spelling would be
a style bug in the kernel or system applications, but it must be used in
application headers since using u_long would depend on namespace pollution.

Newer pollution in sys/types.h includes everything in sys/_pthreadtypes.h,
further pollution for select (pselect(), struct timespec and everything in
sys/timespec.h.  Perhaps current POSIX allows or requires this (I know it
requires pthread stuff, since it standardized some namespace errors).  But
none of this is properly ifdefed, so you get pthread stuff and pselect()
for old POSIXes where they are not permitted in the namespace.

We have a labyrinth of timespec and timeval headers from old attempts to
avoid namespace pollution.  sys/_timespec.h is now nonsense.  Its reason
for existence was to declare struct _timespec for use in contexts where
struct timespec would be pollution.  Now it just declares struct timespec
and time_t.  The layer above this, sys/timespec.h adds just a little to it.
For timevals, there is no intermediate layer (just sys/_timeval.h).

The non-polluting sys/_timespec.h was only used in sys/stat.h, only in
old versions of FreeBSD (FreeBSD-5 and maybe some later versions).  Now
sys/stat.h just uses struct timespec, so it has namespace pollution for
old versions of POSIX.  Old versions of FreeBSD avoided this pollution
by hiding it in !_POSIX_SOURCE ifdefs.  E.g., in FreeBSD-4:

@ #ifndef _POSIX_SOURCE
@ 	struct	timespec st_atimespec;	/* time of last access */
@ 	struct	timespec st_mtimespec;	/* time of last data modification */
@ 	struct	timespec st_ctimespec;	/* time of last file status change */
@ #else
@ 	time_t	  st_atime;		/* time of last access */
@ 	long	  st_atimensec;		/* nsec of last access */
@ 	time_t	  st_mtime;		/* time of last data modification */
@ 	long	  st_mtimensec;		/* nsec of last data modification */
@ 	time_t	  st_ctime;		/* time of last file status change */
@ 	long	  st_ctimensec;		/* nsec of last file status change */
@ #endif

Here st_foo is in the POSIX namespace, but timespec isn't.  There are
serious padding problems with this -- there is little chance that
time_t followed by long has the same padding as struct timespec on all
machine supported now.  struct _timespec was supposed to be used here
to get the same padding, but it never was.  It was only used for padding
of birthtime and/or the end of the struct.

All this to spell timespec's name with an underscore in 1 place.

The _POSIX_SOURCE ifdefs were improved to _BSD_VISIBLE ifdefs in FreeBSD-5,
then lost.

POSIX didn't have timespecs in sys/stat.h even in the 2001 version.
I'm not sure when it was added.  Perhaps never.  Anyway, there are no
ifdefs for it in FreeBSD now.  If it were ifdefed for the POSIX versions
that have it, it would be very obvious that it can't be used for POSIX
versions that don't have it.  The newer POSIX functions in sys/stat.h
are ifdefed, but none of them use timespecs.

Bruce