On Sat, 15 Dec 2012, Garrett Cooper wrote:
On Dec 15, 2012, at 12:34 PM, Mark Johnston wrote:
On Sat, Dec 15, 2012 at 06:55:53PM +0200, Alexander Motin wrote:
Hi.
I'm sorry to interrupt review, but as usual good ideas came during the
final testing, causing another round. :) Here is updated patch for
HEAD, that includes several new changes:
http://people.freebsd.org/~mav/calloutng_12_15.patch
This patch breaks the libprocstat build.
Specifically, the OpenSolaris sys/time.h defines the preprocessor
symbols gethrestime and gethrestime_sec. These symbols are also defined
in cddl/contrib/opensolaris/lib/libzpool/common/sys/zfs_context.h.
libprocstat:zfs.c is compiled using include paths that pick up the
OpenSolaris time.h, and with this patch _callout.h includes sys/time.h.
zfs.c includes taskqueue.h (with _KERNEL defined), which includes
_callout.h, so both time.h and zfs_context.h are included in zfs.c, and
the symbols are thus defined twice.
Gross namespace pollution. sys/_callout.h exists so that the full
namespace pollution of sys/callout.h doesn't get included nested. But
sys/time.h is much more polluted than sys/callout.h.
However, sys/time.h is old standard pollution in sys/param.h, and
sys/callout.h is not so old standard pollution in sys/systm.h. It is
a bug to not include sys/param.h and sys/systm.h in most kernel source
code, so these nested includes are just style bugs -- they have no
effect for correct kernel source code.
The patch below fixes the build for me. Another approach might be to
include sys/_task.h instead of taskqueue.h at the beginning of zfs.c.
Good if it works.
I had a patch open once upon a time to cleanup inclusion of sys/time.h all
over the tree and deal with the sys/time.h <-> time.h pollution issue, but it
got dropped due to lack of interest (20~30 apps/libs were affected IIRC and I only
really got assistance in fixing the UFS and bsnmpd pieces, and gave up due to lack of
response from maintainers). dtrace/zfs is a definite instigator in this pollution (I
remember nasty cddl/... pollution with the compat sys/time.h header).
Please use the unix newline character in mail. The above is difficult to
quote.
The standard sys/time.h pollution in sys/param.h is only in the kernel,
and there aren't many direct includes of sys/time.h in the kernel. Userland
is different and many of the direct includes were correct. But not POSIX
specifies that struct timespec and struct timeval be defined in most places
where they are needed, so the includes of sys/time.h are not necessary
for POSIX or FreeBSD, although FreeBSD man pages still say that they
are necessary. The sys/time.h <-> time.h pollution issue is also only
for userland. Many places depend on one including the other, and include
the wrong one themself.
Bottom line: make sure anything new you're defining isn't already
defined via POSIX or other OSes, and if so please try to make the
implementations match (so that eventual POSIX inclusion might be possible) and
when in doubt I suggest consulting standards@ / brde@.
Bruce
_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
