Date: Sat, 25 Apr 2015 18:47:32 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Garrett Cooper <ngie@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r281966 - head/contrib/netbsd-tests/lib/libpthread Message-ID: <20150425175312.T972@besplex.bde.org> In-Reply-To: <201504250430.t3P4U1FR044477@svn.freebsd.org> References: <201504250430.t3P4U1FR044477@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 25 Apr 2015, Garrett Cooper wrote:
> Log:
> Add #include sys/types.h for register_t for mips
Why not fix mips?
> Modified: head/contrib/netbsd-tests/lib/libpthread/t_swapcontext.c
> ==============================================================================
> --- head/contrib/netbsd-tests/lib/libpthread/t_swapcontext.c Sat Apr 25 01:29:40 2015 (r281965)
> +++ head/contrib/netbsd-tests/lib/libpthread/t_swapcontext.c Sat Apr 25 04:30:01 2015 (r281966)
> @@ -28,6 +28,9 @@
> #include <sys/cdefs.h>
> __RCSID("$NetBSD");
>
> +#ifdef __FreeBSD__
> +#include <sys/types.h>
> +#endif
> #include <pthread.h>
> #include <ucontext.h>
> #include <stdio.h>
The test detected that ucontext.h is broken on mips, but was shot instead
of mips.
<ucontext.h> has massive namespace pollution, but not enough to declare
register_t. /usr/include/ucontext.h has its own namespace pollution,
and it includes machine/ucontext.h which has massively more on some
arches.
i386/include/ucontext.h used to be careful about this. It used the
basic type corresponding to register_t (int) to avoid any dependencies.
amd64/include/ucontext.h was and is not so careful. It used and uses
__register_t, so it depends on <sys/_types.h> or <machine/_types.h>
though not on <sys/types.h>. i386 ucontext.h has been unimproved by
merging with amd64. Now the basic type cannot be used, since
register_t is MD and in fact varies between i386 and amd64. i386
ucontext.h uses __register_t even its own compatibility part that
cannot be merged.
mips is not careful about this. Its ucontext.h uses register_t. Its
ucontext.h has many even larger bugs.
The non-broken MD ucontext.h's don't include any _types.h file to
get __register_t defined, but that is correct since it is an error
to include <machine/ucontext.h> directly except possibly in the
kernel. Applications must include <ucontext.h>
Nearby namespace pollution: only uc_* and ss_* are reserved in POSIX
<ucontext.h>, at least in old versions, but FreeBSD has the following:
- some at the <uncontext.h>. See the enclosed patch which does't
really fix it.
- arm: seems to be perfectly correct. It uses underscores a lot.
It is just ugly to use "unsigned int" instead of something shorter.
The BSD spelling u_int cannot be used in standard user headers since
it would be namespace pollution. __register_t isn't really right
on i386 or amd64, since this is one of the few places where unsigned
types should be used. Using plain int for __register_t is just an
old bug in i386 (from 386BSD?) copied to amd64.
- arm64: very broken. It never uses underscores, so pollutes the
following the following namespaces: gp*, gp*, mc_*. It also uses
u_int, uint32_t and uint64_t. Better yet, it uses the long long
abomination instead of uint64_t in some places.
- mips: mc_*, sr (a field in struct __mcontext that doesn't even have
an mc_ prefix), mullo, mulhi (like sr), register_t, f_register_t,
int32_t, uint32_t, mcontext32_t (only mcontext_t is allowed (and
required, but pollution in the mc* namespace is limited), SZREG,
UCTX_REG(), UCR_*.
- powerpc: mc_*, uint32_t, register_t, uint64_t (POSIX allows foo_t to
be declared, but only a low quality implementation would declare it).
- sparc64: uint64_t, mc_*
- x86: mc_*. As well as __register_t, x86 is careful to use __uintN_t
instead of uintN_t.
Old changes to document and partially reduce the pollution at the top
level:
X Index: ucontext.h
X ===================================================================
X RCS file: /home/ncvs/src/sys/sys/ucontext.h,v
X retrieving revision 1.11
X diff -u -2 -r1.11 ucontext.h
X --- ucontext.h 9 Nov 2003 20:31:04 -0000 1.11
X +++ ucontext.h 10 Nov 2003 09:38:33 -0000
X @@ -32,5 +32,11 @@
X #define _SYS_UCONTEXT_H_
X
X +#ifdef _KERNEL
X +#include <sys/_sigset.h>
X +#else
X +/* XXX need stack_t as well as sigset_t. */
X #include <sys/signal.h>
X +#endif
X +
X #include <machine/ucontext.h>
X
POSIX requires stack_t, and in old versions didn't alow anything else
from <signal.h>. Newer versions of POSIX tend to break things like this
by allowing but not requiring all the symbols in a header like <signal.h>
X @@ -50,6 +56,7 @@
X stack_t uc_stack;
X int uc_flags;
X +/* XXX namespace pollution. */
X #define UCF_SWAPPED 0x00000001 /* Used by swapcontext(3). */
X - int __spare__[4];
X + int uc_spare[4];
X } ucontext_t;
X
Names like UCF_SWAPPED bogotify more careful naming for _MC*. So do
names the mc_*.
All the underscores for __spare__ are bogus, and without a uc_ prefix
it isn't clear what this field is spare for.
X @@ -61,27 +68,22 @@
X struct ucontext4 *uc_link;
X stack_t uc_stack;
X - int __spare__[8];
X + int uc_spare[8];
X };
X -#else /* __i386__ || __alpha__ */
X +#else /* __i386__ || __alpha__ */
X #define ucontext4 ucontext
X -#endif /* __i386__ || __alpha__ */
X -#endif /* _KERNEL */
X +#endif /* __i386__ || __alpha__ */
X +#endif /* _KERNEL */
X
X #ifndef _KERNEL
X -
X __BEGIN_DECLS
X -
X int getcontext(ucontext_t *);
X -int setcontext(const ucontext_t *);
X void makecontext(ucontext_t *, void (*)(void), int, ...);
X -int signalcontext(ucontext_t *, int, __sighandler_t *);
X +int setcontext(const ucontext_t *);
X +int signalcontext(ucontext_t *, int, void (*)(int));
X int swapcontext(ucontext_t *, const ucontext_t *);
X -
X __END_DECLS
X
X #else /* _KERNEL */
X
X -struct thread;
X -
X /*
X * Flags for get_mcontext(). The low order 4 bits (i.e a mask of 0x0f) are
X @@ -91,8 +93,8 @@
X #define GET_MC_CLEAR_RET 1
X
X -/* Machine-dependent functions: */
X +struct thread;
X +
X int get_mcontext(struct thread *, mcontext_t *, int);
X int set_mcontext(struct thread *, const mcontext_t *);
X -
X #endif /* !_KERNEL */
X
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150425175312.T972>