From owner-freebsd-arch@FreeBSD.ORG Wed Jul 9 10:48:21 2014 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 7FA8C349; Wed, 9 Jul 2014 10:48:21 +0000 (UTC) Received: from mail109.syd.optusnet.com.au (mail109.syd.optusnet.com.au [211.29.132.80]) by mx1.freebsd.org (Postfix) with ESMTP id 450722CE1; Wed, 9 Jul 2014 10:48:20 +0000 (UTC) Received: from c122-106-147-133.carlnfd1.nsw.optusnet.com.au (c122-106-147-133.carlnfd1.nsw.optusnet.com.au [122.106.147.133]) by mail109.syd.optusnet.com.au (Postfix) with ESMTPS id 2CA76D65DDE; Wed, 9 Jul 2014 20:48:13 +1000 (EST) Date: Wed, 9 Jul 2014 20:48:12 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Bryan Drewery Subject: Re: sys/proc.h inclusion of sys/time.h In-Reply-To: <53BC4F49.7000903@FreeBSD.org> Message-ID: <20140709201148.W1201@besplex.bde.org> References: <53BC4F49.7000903@FreeBSD.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=QIpRGG7L c=1 sm=1 tr=0 a=7NqvjVvQucbO2RlWB8PEog==:117 a=PO7r1zJSAAAA:8 a=z5S34nFNDvQA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=8txysXnHcTpGYzNcLocA:9 a=CjuIK1q_8ugA:10 Cc: arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Jul 2014 10:48:21 -0000 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