Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 31 Jan 2012 07:30:15 GMT
From:      Bruce Evans <brde@optusnet.com.au>
To:        freebsd-bugs@FreeBSD.org
Subject:   Re: kern/164656: Add size_t declaration to ucontext.h of 10-CURRENT
Message-ID:  <201201310730.q0V7UFM5060733@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/164656; it has been noted by GNATS.

From: Bruce Evans <brde@optusnet.com.au>
To: Jyun-Yan You <jyyou@cs.nctu.edu.tw>
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 <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?201201310730.q0V7UFM5060733>