Bug 164656

Comment 1 Bruce Evans 2012-01-31 07:19:48 UTC

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

Comment 2 Eitan Adler 2018-05-20 23:50:52 UTC

For bugs matching the following conditions:
- Status == In Progress
- Assignee == "bugs@FreeBSD.org"
- Last Modified Year <= 2017

Do
- Set Status to "Open"

Comment 3 Graham Perrin 2022-10-17 12:40:41 UTC

Keyword: 

    patch
or  patch-ready

– in lieu of summary line prefix: 

    [patch]

* bulk change for the keyword
* summary lines may be edited manually (not in bulk). 

Keyword descriptions and search interface: 

    <https://bugs.freebsd.org/bugzilla/describekeywords.cgi>