[PATCH 2/3] Enforce global atexit handling
Jeff Johnston
jjohnstn@redhat.com
Tue May 7 19:05:00 GMT 2013
I don't see why you are removing the atexit code from __reclaim_reent
and _wrapup_reent. If someone is using the fields for thread-specific
atexit handling, let them. You are not removing the fields with this
change.
I would prefer you simply document that atexit / exit only uses the
_GLOBAL_REENT struct which is true.
In your follow-up patch, I don't like
_REENT_NEGLECT_BINARY_COMPATIBILITY because that has more far-reaching
meaning than what you are actually changing. Just use _REENT_NEW_ATEXIT
or _REENT_GLOBAL_ATEXIT or whatever as long as it pertains to atexit and
then modify however you wish, including removing the fields from the
reent struct and thus from __reclaim_reent and _wrapup_reent.
In the future we can make this the default behaviour.
Everything else is fine.
-- Jeff J.
On 05/04/2013 10:12 AM, Sebastian Huber wrote:
> This patch removes the atexit handling from struct _reent. The two
> atexit fields are provided for backward binary compatibility. The
> fields are used only in the statically allocated _GLOBAL_ATEXIT for the
> global atexit handling.
>
> newlib/ChangeLog
> 2013-05-04 Sebastian Huber <sebastian.huber@embedded-brains.de>
>
> * libc/include/sys/reent.h (_ATEXIT_INIT): Define.
> (_ATEXIT_INIT_PTR): Likewise.
> (_REENT_INIT_ATEXIT): Likewise.
> (_REENT_INIT_ATEXIT_PTR): Likewise.
> (_GLOBAL_ATEXIT): Likewise.
> * libc/reent/reent.c (_reclaim_reent): Remove atexit cleanup.
> (_wrapup_reent): Remove atexit handling.
> * libc/stdlib/__atexit.c (_GLOBAL_ATEXIT0): Define.
> (__register_exitproc): Use _GLOBAL_ATEXIT and _GLOBAL_ATEXIT0.
> * libc/stdlib/__call_atexit.c (__call_exitprocs): Likewise.
> ---
> newlib/libc/include/sys/reent.h | 42 +++++++++++++++++++++++++-----------
> newlib/libc/reent/reent.c | 23 --------------------
> newlib/libc/stdlib/__atexit.c | 10 +++++----
> newlib/libc/stdlib/__call_atexit.c | 4 ++--
> 4 files changed, 37 insertions(+), 42 deletions(-)
>
> diff --git a/newlib/libc/include/sys/reent.h b/newlib/libc/include/sys/reent.h
> index ff0242e..cf2969d 100644
> --- a/newlib/libc/include/sys/reent.h
> +++ b/newlib/libc/include/sys/reent.h
> @@ -85,6 +85,12 @@ struct _atexit {
> void (*_fns[_ATEXIT_SIZE])(void); /* the table itself */
> struct _on_exit_args * _on_exit_args_ptr;
> };
> +# define _ATEXIT_INIT {_NULL, 0, {_NULL}, _NULL}
> +# define _ATEXIT_INIT_PTR(var) \
> + (var)->_next = _NULL; \
> + (var)->_ind = 0; \
> + (var)->_fns[0] = _NULL; \
> + (var)->_on_exit_args_ptr = _NULL
> #else
> struct _atexit {
> struct _atexit *_next; /* next in list */
> @@ -93,8 +99,20 @@ struct _atexit {
> void (*_fns[_ATEXIT_SIZE])(void); /* the table itself */
> struct _on_exit_args _on_exit_args;
> };
> +# define _ATEXIT_INIT {_NULL, 0, {_NULL}, {{_NULL}, {_NULL}, 0, 0}}
> +# define _ATEXIT_INIT_PTR(var) \
> + (var)->_next = _NULL; \
> + (var)->_ind = 0; \
> + (var)->_fns[0] = _NULL; \
> + (var)->_on_exit_args._fntypes = 0; \
> + (var)->_on_exit_args._fnargs[0] = _NULL
> #endif
>
> +#define _REENT_INIT_ATEXIT \
> + _NULL, _ATEXIT_INIT,
> +#define _REENT_INIT_ATEXIT_PTR(var, var0) \
> + (var)->_atexit = _NULL; _ATEXIT_INIT_PTR(var0);
> +
> /*
> * Stdio buffers.
> *
> @@ -392,9 +410,10 @@ struct _reent
> /* signal info */
> void (**(_sig_func))(int);
>
> - /* atexit stuff */
> - struct _atexit *_atexit;
> - struct _atexit _atexit0;
> + /* The next two fields are used for atexit stuff. They are used only in
> + _GLOBAL_REENT, but not in other struct _reent instances. */
> + struct _atexit *_atexit; /* points to head of LIFO stack */
> + struct _atexit _atexit0; /* one guaranteed table, required by ANSI */
>
> struct _glue __sglue; /* root of glue chain */
> __FILE *__sf; /* file descriptors */
> @@ -425,8 +444,7 @@ extern const struct __sFILE_fake __sf_fake_stderr;
> _NULL, \
> _NULL, \
> _NULL, \
> - _NULL, \
> - {_NULL, 0, {_NULL}, _NULL}, \
> + _REENT_INIT_ATEXIT \
> {_NULL, 0, _NULL}, \
> _NULL, \
> _NULL, \
> @@ -452,11 +470,7 @@ extern const struct __sFILE_fake __sf_fake_stderr;
> (var)->_localtime_buf = _NULL; \
> (var)->_asctime_buf = _NULL; \
> (var)->_sig_func = _NULL; \
> - (var)->_atexit = _NULL; \
> - (var)->_atexit0._next = _NULL; \
> - (var)->_atexit0._ind = 0; \
> - (var)->_atexit0._fns[0] = _NULL; \
> - (var)->_atexit0._on_exit_args_ptr = _NULL; \
> + _REENT_INIT_ATEXIT_PTR(var, &(var)->_atexit0) \
> (var)->__sglue._next = _NULL; \
> (var)->__sglue._niobs = 0; \
> (var)->__sglue._iobs = _NULL; \
> @@ -641,7 +655,8 @@ struct _reent
> } _unused;
> } _new;
>
> - /* atexit stuff */
> + /* The next two fields are used for atexit stuff. They are used only in
> + _GLOBAL_REENT, but not in other struct _reent instances. */
> struct _atexit *_atexit; /* points to head of LIFO stack */
> struct _atexit _atexit0; /* one guaranteed table, required by ANSI */
>
> @@ -698,8 +713,7 @@ struct _reent
> {0, {0}} \
> } \
> }, \
> - _NULL, \
> - {_NULL, 0, {_NULL}, {{_NULL}, {_NULL}, 0, 0}}, \
> + _REENT_INIT_ATEXIT \
> _NULL, \
> {_NULL, 0, _NULL} \
> }
> @@ -791,6 +805,8 @@ void _reclaim_reent _PARAMS ((struct _reent *));
>
> #define _GLOBAL_REENT _global_impure_ptr
>
> +#define _GLOBAL_ATEXIT (_GLOBAL_REENT->_atexit)
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/newlib/libc/reent/reent.c b/newlib/libc/reent/reent.c
> index 63812db..3dc50aa 100644
> --- a/newlib/libc/reent/reent.c
> +++ b/newlib/libc/reent/reent.c
> @@ -87,20 +87,6 @@ _DEFUN (_reclaim_reent, (ptr),
> _free_r (ptr, ptr->_localtime_buf);
> if (ptr->_asctime_buf)
> _free_r (ptr, ptr->_asctime_buf);
> - if (ptr->_atexit && ptr->_atexit->_on_exit_args_ptr)
> - _free_r (ptr, ptr->_atexit->_on_exit_args_ptr);
> -#else
> - /* atexit stuff */
> - if ((ptr->_atexit) && (ptr->_atexit != &ptr->_atexit0))
> - {
> - struct _atexit *p, *q;
> - for (p = ptr->_atexit; p != &ptr->_atexit0;)
> - {
> - q = p;
> - p = p->_next;
> - _free_r (ptr, q);
> - }
> - }
> #endif
>
> if (ptr->_cvtbuf)
> @@ -131,20 +117,11 @@ _DEFUN (_reclaim_reent, (ptr),
> void
> _DEFUN (_wrapup_reent, (ptr), struct _reent *ptr)
> {
> - register struct _atexit *p;
> register int n;
>
> if (ptr == NULL)
> ptr = _REENT;
>
> -#ifdef _REENT_SMALL
> - for (p = ptr->_atexit, n = p ? p->_ind : 0; --n >= 0;)
> - (*p->_fns[n]) ();
> -#else
> - for (p = ptr->_atexit; p; p = p->_next)
> - for (n = p->_ind; --n >= 0;)
> - (*p->_fns[n]) ();
> -#endif
> if (ptr->__cleanup)
> (*ptr->__cleanup) (ptr);
> }
> diff --git a/newlib/libc/stdlib/__atexit.c b/newlib/libc/stdlib/__atexit.c
> index 1caf2e5..a095313 100644
> --- a/newlib/libc/stdlib/__atexit.c
> +++ b/newlib/libc/stdlib/__atexit.c
> @@ -15,6 +15,8 @@ void * malloc(size_t) _ATTRIBUTE((__weak__));
> extern _LOCK_RECURSIVE_T __atexit_lock;
> #endif
>
> +#define _GLOBAL_ATEXIT0 (&_GLOBAL_REENT->_atexit0)
> +
> /*
> * Register a function to be performed at exit or on shared library unload.
> */
> @@ -34,9 +36,9 @@ _DEFUN (__register_exitproc,
> __lock_acquire_recursive(__atexit_lock);
> #endif
>
> - p = _GLOBAL_REENT->_atexit;
> + p = _GLOBAL_ATEXIT;
> if (p == NULL)
> - _GLOBAL_REENT->_atexit = p = &_GLOBAL_REENT->_atexit0;
> + _GLOBAL_ATEXIT = p = _GLOBAL_ATEXIT0;
> if (p->_ind >= _ATEXIT_SIZE)
> {
> #ifndef _ATEXIT_DYNAMIC_ALLOC
> @@ -56,8 +58,8 @@ _DEFUN (__register_exitproc,
> return -1;
> }
> p->_ind = 0;
> - p->_next = _GLOBAL_REENT->_atexit;
> - _GLOBAL_REENT->_atexit = p;
> + p->_next = _GLOBAL_ATEXIT;
> + _GLOBAL_ATEXIT = p;
> #ifndef _REENT_SMALL
> p->_on_exit_args._fntypes = 0;
> p->_on_exit_args._is_cxa = 0;
> diff --git a/newlib/libc/stdlib/__call_atexit.c b/newlib/libc/stdlib/__call_atexit.c
> index a21dd57..76d3f12 100644
> --- a/newlib/libc/stdlib/__call_atexit.c
> +++ b/newlib/libc/stdlib/__call_atexit.c
> @@ -76,8 +76,8 @@ _DEFUN (__call_exitprocs, (code, d),
>
> restart:
>
> - p = _GLOBAL_REENT->_atexit;
> - lastp = &_GLOBAL_REENT->_atexit;
> + p = _GLOBAL_ATEXIT;
> + lastp = &_GLOBAL_ATEXIT;
> while (p)
> {
> #ifdef _REENT_SMALL
>
More information about the Newlib
mailing list