This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/3] Enforce global atexit handling

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
Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]