From owner-freebsd-bugs@FreeBSD.ORG Tue Jan 31 07:30:16 2012 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1BC02106566C for ; Tue, 31 Jan 2012 07:30:16 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id DCD688FC19 for ; Tue, 31 Jan 2012 07:30:15 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q0V7UFWd060736 for ; Tue, 31 Jan 2012 07:30:15 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.5/8.14.5/Submit) id q0V7UFM5060733; Tue, 31 Jan 2012 07:30:15 GMT (envelope-from gnats) Date: Tue, 31 Jan 2012 07:30:15 GMT Message-Id: <201201310730.q0V7UFM5060733@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Bruce Evans Cc: Subject: Re: kern/164656: Add size_t declaration to ucontext.h of 10-CURRENT X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Bruce Evans List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 31 Jan 2012 07:30:16 -0000 The following reply was made to PR kern/164656; it has been noted by GNATS. From: Bruce Evans To: Jyun-Yan You Cc: FreeBSD-gnats-submit@freebsd.org, freebsd-bugs@freebsd.org Subject: Re: kern/164656: Add size_t declaration to ucontext.h of 10-CURRENT Date: Tue, 31 Jan 2012 18:19:48 +1100 (EST) 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 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 % +#else % +/* XXX need stack_t as well as sigset_t. */ % #include % +#endif % + % #include % % @@ -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