Date: Tue, 31 Jan 2012 18:19:48 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Jyun-Yan You <jyyou@cs.nctu.edu.tw> Cc: freebsd-bugs@freebsd.org, FreeBSD-gnats-submit@freebsd.org Subject: Re: kern/164656: Add size_t declaration to ucontext.h of 10-CURRENT Message-ID: <20120131173729.A1450@besplex.bde.org> In-Reply-To: <201201310543.q0V5hPOn019008@csduty.cs.nctu.edu.tw> References: <201201310543.q0V5hPOn019008@csduty.cs.nctu.edu.tw>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 31 Jan 2012, Jyun-Yan You wrote: >> Description: > ucontext.h does not include any header file that defines size_t. > If we write a program that includes sys/ucontext.h, it may cause compilation errors. > ports/164654 is the real case. -current added 2 new prototypes with 5 bugs altogether; 3 style bugs and 2 namespace bugs: - 2 indentation errors. Old prototypes use normal indentation. - 1 named parameter. Old prototypes don't use named parameters. - the name of the parameter pollutes the application namespace - size_t is used, but is not defined. Elsewhere (for stack_t), size_t is avoided by using __size_t for stack_t, but there is gross namespace pollution from including <sys/signal.h> for nothing more than stack_t. > --- ucontext.h.diff begins here --- > --- sys/sys/ucontext.h.orig 2012-01-31 12:50:57.702804441 +0800 > +++ sys/sys/ucontext.h 2012-01-31 12:51:26.217639882 +0800 > @@ -69,6 +69,11 @@ > > #ifndef _KERNEL > > +#ifndef _SIZE_T_DECLARED > +typedef __size_t size_t; > +#define _SIZE_T_DECLARED > +#endif > + > __BEGIN_DECLS > > int getcontext(ucontext_t *); > --- ucontext.h.diff ends here --- size_t is only used (for 1 of the new prototypes) if __BSD_VISIBLE. That prototype's function is named with 2 underscores, so it is not usable by applications, so the prototype might as well use 2 underscores to hide __size_t too. Otherwise, the above ifdef should be conditional on __BSD_VISIBLE too. Here is an old patch to reduce and document some pollution, and to fix some style bugs. % Index: ucontext.h % =================================================================== % RCS file: /home/ncvs/src/sys/sys/ucontext.h,v % retrieving revision 1.11 % diff -u -2 -r1.11 ucontext.h % --- ucontext.h 9 Nov 2003 20:31:04 -0000 1.11 % +++ ucontext.h 10 Nov 2003 09:38:33 -0000 % @@ -32,5 +32,11 @@ % #define _SYS_UCONTEXT_H_ % % +#ifdef _KERNEL % +#include <sys/_sigset.h> % +#else % +/* XXX need stack_t as well as sigset_t. */ % #include <sys/signal.h> % +#endif % + % #include <machine/ucontext.h> % % @@ -50,6 +56,7 @@ % stack_t uc_stack; % int uc_flags; % +/* XXX namespace pollution. */ % #define UCF_SWAPPED 0x00000001 /* Used by swapcontext(3). */ % - int __spare__[4]; % + int uc_spare[4]; % } ucontext_t; % % @@ -61,27 +68,22 @@ % struct ucontext4 *uc_link; % stack_t uc_stack; % - int __spare__[8]; % + int uc_spare[8]; % }; % -#else /* __i386__ || __alpha__ */ % +#else /* __i386__ || __alpha__ */ % #define ucontext4 ucontext % -#endif /* __i386__ || __alpha__ */ % -#endif /* _KERNEL */ % +#endif /* __i386__ || __alpha__ */ % +#endif /* _KERNEL */ % % #ifndef _KERNEL % - % __BEGIN_DECLS % - % int getcontext(ucontext_t *); % -int setcontext(const ucontext_t *); % void makecontext(ucontext_t *, void (*)(void), int, ...); % -int signalcontext(ucontext_t *, int, __sighandler_t *); % +int setcontext(const ucontext_t *); % +int signalcontext(ucontext_t *, int, void (*)(int)); % int swapcontext(ucontext_t *, const ucontext_t *); % - % __END_DECLS % % #else /* _KERNEL */ % % -struct thread; % - % /* % * Flags for get_mcontext(). The low order 4 bits (i.e a mask of 0x0f) are % @@ -91,8 +93,8 @@ % #define GET_MC_CLEAR_RET 1 % % -/* Machine-dependent functions: */ % +struct thread; % + % int get_mcontext(struct thread *, mcontext_t *, int); % int set_mcontext(struct thread *, const mcontext_t *); % - % #endif /* !_KERNEL */ % Further investigation shows that size_t should be declared unconditionally anyway, for stack_t in both signal.h and ucontext.h. POSIX reqires "stack_t uc_stack" in ucontext_t (this bogusly also requires ucontext_t to be highly non-opaque, and in fact a struct with certain members), and it requires stack_t to be as in signal.h. And it requires "size_t ss_size" in stack_t (this bogusly also requires stack_t to be highly non-opaque, and in fact a struct with certain members). stack_t and ucontext.h are XSI extensions, so their namespace errors are not The history of this is not completely clear, but some of it is: - in POSIX.1-2001, stack_t and ucontext.h were only XSI extensions. XSI even required signal.h to declare ucontext_t! FreeBSD never implemented the latter. - in POSIX-1.2007-draft, ucontext.h no longer exists. stack_t and ucontext_t are now non-optional (C extensions instead of an XSI extensions), and are required to be implemented in signal.h. FreeBSD doesn't seem to have caught up with this. stack_t is still only used by sigaltstack(), which is still only an XSI extension. But to go with this pollution, size_t is now explicitly required to be declared in signal.h. So FreeBSD's careful avoidance of this pollution is now non-conforming :-(. FreeBSD clearly hasn't caught up with this. It still declares stack_t in signal.h conditionally on __XSI_VISIBLE (which is the default). sigaltstack() and struct sigalstack are standard in BSD, but the stack_t mistake isn't. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120131173729.A1450>