sys/proc.h inclusion of sys/time.h

Bruce Evans brde at optusnet.com.au
Wed Jul 9 10:48:21 UTC 2014


On Tue, 8 Jul 2014, Bryan Drewery wrote:

> In r34924 sys/proc.h was changed to only include sys/time.h if not building 
> in kernel.

That should give enough namespace pollution for your purposes.  Several
other non-kernel abuses depend on it.  The ifdef to not do it in the
kernel is to depend on the standard namespace pollution that sys/time.h
is included in sys/param.h.

> However, as the comment next to time.h says itimerval is needed.

I don't like comments like that, since they will rot as depenendcies
on the pollution grow.  I must have written it since I hoped to remove
the sys/time.h pollution on sys/param.h soon.

> struct proc {
> ..
> struct itimerval p_realtimer;   /* (c) Alarm timer. */
>
> This manifests when (hackishly) including sys/proc.h with _KERNEL defined:
>
>> In file included from 
>> /root/svn/base/usr.sbin/tcpdump/tcpdump/../../../contrib/tcpdump/print-pflog.c:37:
>> /usr/include/sys/proc.h:524:19: error: field has incomplete type 'struct 
>> itimerval'
>>         struct itimerval p_realtimer;   /* (c) Alarm timer. */
>
> (Why am I doing this? I need PID_MAX and NO_PID for a tcpdump change I am 
> testing that is intended for upstreaming. Perhaps I can use kern.pid_max in 
> __FreeBSD__ and other hacks on other platforms, I have not yet decided on 
> this.)

Ah, you were chummy with the implementation, but not chummy enough to
know all the details of the kernel environment that must be duplicated
to use the hack of defining _KERNEL.  It seems to be necessary to include
sys/param.h and define _KERNEL before that.  There may be collateral
pollution and further chumminess to avoid problems with it.

Hrmph, PID_MAX actually is a parameter, so in belongs in sys/param.h
unlike most of the things there.  I thought it was in POSIX.  POSIX
actually considered and rejected it since it is incompatible with
opaque pid_t.

> Should we move the inclusion of sys/time.h outside of this ifdef or just add 
> a forward declaration for struct itimerval above struct proc like many 
> others?

Moving it would be a step backwards.  Elsewhere in the file, I tried hard
to keep the rusage struct out of it.  My version has a struct rusage_ext
with all times it it uint64_t except for one struct bintime.  This one
struct bintime gives a much more critical dependency on sys/time.h
than the rotted comment says.  -current instead just includes
sys/resource.h.  This gives lots of pollution, but not sys/time.h since
sys/resource.h has been de-polluted to include only sys/_timeval.h to
declare the struct timeval's that it uses.

Forward declarations only work for incomplete types.  Lots of little
include files like sys/_timeval.h can be used to reduce pollution.
I don't like this much since using just a few of these wastes more
time than including one huge polluted file; it just gives less
pollution.  sys/_timeval.h and sys/_timespec are still clean, but
sys/timespec.h has rotted into 2 layers of nesting plus internal
pollution.

Bruce


More information about the freebsd-arch mailing list