RFA: Reducing the size of struct _reent by not supporting on_exit()

J. Johnston jjohnstn@redhat.com
Fri Jun 6 05:48:00 GMT 2003


Nick Clifton wrote:
> Hi Jeff,
> 
> 
>>>I would think a better tack would be a macro that changes the _fns
>>>from being:
>>>	void	(*_fns[_ATEXIT_SIZE])(void);	/* the table itself */
>>>to:
>>>	void	**_fns;				/* the table itself */
>>>and then allocate it if the user ever needed on_exit functionality.
>>>Obviously
>>>you would have to check whether it was allocated or not.
>>>
>>
>>I agree.  A dynamic allocation solution makes more sense for both the
>>function pointers and the argument list.  If a user doesn't call
>>on_exit, then there is no reason to allocate the additional area.  If
>>an application needs to use it, there is no reason to fail
>>unconditionally.  This fits in with the whole spirit of _REENT_SMALL.
> 
> 
> Ok - how about this revised patch then ?  Can I apply this one ?

Better.  Have you considered making the atexit function list also
dynamic?  If you are keen to save 128 bytes for an application not
using on_exit, then it kind of makes sense not to allocate the array
for atexit functions unless atexit() gets called.

Note that either change will require rebuilding the xstormy16 newlib
because the newlib Makefiles do not properly have the dependency between
files that use REENT stuff and the sys/reent.h header file.  In this case,
you are changing the offsets of the fields after _atexit (I/O in particular).

I also have a few minor comments.  There appears to be a few coding indentation
mixups in the patch below.  It might be caused by tabs, but please verify
that your code is correct (a number of returns after if statements are incorrect).

In exit.c, if you are going to code a "for statement" in a macro, then I would
like to see you bracket the statements below the macro usage and use indentation so it is clear
what code is running inside it.

-- Jeff J.

> 
> Cheers
>         Nick
> 
> 2003-06-05  Nick Clifton  <nickc@redhat.com>
> 
> 	* libc/include/sys/reent.h (struct _on_exit_args): New
> 	structure containing fields used by the on_exit() function.
>         (struct _atexit): Include struct _on_exit_args.  For
> 	_REENT_SMALL do his via a pointer that is initialised when
> 	needed.
>         * libc/reent/reent.c (_reclaim_reent): Free the _on_exit_args
> 	structure, if one has been allocated.
>         * libc/stdlib/atexit.c (atexit): Update indirection to
> 	_fntypes field.
>         * libc/stdlib/on_exit.c (on_exit): Indirect via the
> 	_on_exit_args structure.  For _REENT_SMALL, allocate a
> 	structure if one does not exist.
>         * libc/stdlib/exit.c (exit): Indirect via the _on_exit_args
> 	structure.
> 
> Index: newlib/libc/include/sys/reent.h
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/include/sys/reent.h,v
> retrieving revision 1.22
> diff -c -3 -p -r1.22 reent.h
> *** newlib/libc/include/sys/reent.h	7 Mar 2003 15:56:49 -0000	1.22
> --- newlib/libc/include/sys/reent.h	5 Jun 2003 09:28:35 -0000
> *************** struct __tm
> *** 64,83 ****
>   
>   #define	_ATEXIT_SIZE 32	/* must be at least 32 to guarantee ANSI conformance */
>   
> ! #ifndef _REENT_SMALL
>   struct _atexit {
> - 	struct	_atexit *_next;			/* next in list */
>   	int	_ind;				/* next index in this table */
>   	void	(*_fns[_ATEXIT_SIZE])(void);	/* the table itself */
> ! 	void	*_fnargs[_ATEXIT_SIZE];	        /* fn args for on_exit */
> ! 	__ULong _fntypes;           	        /* type of exit routine */
>   };
>   #else
>   struct _atexit {
>   	int	_ind;				/* next index in this table */
>   	void	(*_fns[_ATEXIT_SIZE])(void);	/* the table itself */
> ! 	void	*_fnargs[_ATEXIT_SIZE];	        /* fn args for on_exit */
> ! 	__ULong _fntypes;           	        /* type of exit routine */
>   };
>   #endif
>   
> --- 64,87 ----
>   
>   #define	_ATEXIT_SIZE 32	/* must be at least 32 to guarantee ANSI conformance */
>   
> ! struct _on_exit_args {
> ! 	void *  _fnargs[_ATEXIT_SIZE];	        /* fn args for on_exit */
> ! 	__ULong _fntypes;           	        /* type of exit routine -
> ! 						   Must have at least _ATEXIT_SIZE bits */
> ! };
> ! 
> ! #ifdef _REENT_SMALL
>   struct _atexit {
>   	int	_ind;				/* next index in this table */
>   	void	(*_fns[_ATEXIT_SIZE])(void);	/* the table itself */
> !       struct _on_exit_args * _on_exit_args_ptr;
>   };
>   #else
>   struct _atexit {
> + 	struct	_atexit *_next;			/* next in list */
>   	int	_ind;				/* next index in this table */
>   	void	(*_fns[_ATEXIT_SIZE])(void);	/* the table itself */
> !       struct _on_exit_args _on_exit_args;
>   };
>   #endif
>   
> Index: newlib/libc/reent/reent.c
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/reent/reent.c,v
> retrieving revision 1.5
> diff -c -3 -p -r1.5 reent.c
> *** newlib/libc/reent/reent.c	3 Jun 2003 19:48:07 -0000	1.5
> --- newlib/libc/reent/reent.c	5 Jun 2003 09:28:35 -0000
> *************** _DEFUN (_reclaim_reent, (ptr),
> *** 81,86 ****
> --- 81,88 ----
>   	_free_r (ptr, ptr->_localtime_buf);
>         if (ptr->_asctime_buf)
>   	_free_r (ptr, ptr->_asctime_buf);
> +       if (ptr->_atexit._on_exit_args_ptr)
> + 	_free_r (ptr->_atexit._on_exit_args_ptr);
>   #else
>         /* atexit stuff */
>         if ((ptr->_atexit) && (ptr->_atexit != &ptr->_atexit0))
> 
> Index: newlib/libc/stdlib/atexit.c
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/stdlib/atexit.c,v
> retrieving revision 1.3
> diff -c -3 -p -r1.3 atexit.c
> *** newlib/libc/stdlib/atexit.c	15 May 2002 22:58:10 -0000	1.3
> --- newlib/libc/stdlib/atexit.c	5 Jun 2003 09:28:36 -0000
> *************** _DEFUN (atexit,
> *** 74,80 ****
>         if ((p = (struct _atexit *) malloc (sizeof *p)) == NULL)
>   	return -1;
>         p->_ind = 0;
> !       p->_fntypes = 0;
>         p->_next = _REENT->_atexit;
>         _REENT->_atexit = p;
>       }
> --- 74,80 ----
>         if ((p = (struct _atexit *) malloc (sizeof *p)) == NULL)
>   	return -1;
>         p->_ind = 0;
> !       p->_on_exit_args._fntypes = 0;
>         p->_next = _REENT->_atexit;
>         _REENT->_atexit = p;
>       }
> 
> Index: newlib/libc/stdlib/on_exit.c
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/stdlib/on_exit.c,v
> retrieving revision 1.1
> diff -c -3 -p -r1.1 on_exit.c
> *** newlib/libc/stdlib/on_exit.c	15 May 2002 22:58:10 -0000	1.1
> --- newlib/libc/stdlib/on_exit.c	5 Jun 2003 09:28:36 -0000
> *************** _DEFUN (on_exit,
> *** 68,78 ****
>   	_VOID _EXFUN ((*fn), (int, _PTR)) _AND
>           _PTR arg)
>   {
>     register struct _atexit *p;
>     void (*x)(void) = (void (*)(void))fn;
>   
>   /* _REENT_SMALL on_exit() doesn't allow more than the required 32 entries.  */
> ! #ifndef _REENT_SMALL
>     if ((p = _REENT->_atexit) == NULL)
>       _REENT->_atexit = p = &_REENT->_atexit0;
>     if (p->_ind >= _ATEXIT_SIZE)
> --- 68,92 ----
>   	_VOID _EXFUN ((*fn), (int, _PTR)) _AND
>           _PTR arg)
>   {
> +   struct _on_exit_args * args;
>     register struct _atexit *p;
>     void (*x)(void) = (void (*)(void))fn;
>   
>   /* _REENT_SMALL on_exit() doesn't allow more than the required 32 entries.  */
> ! #ifdef _REENT_SMALL
> !   p = &_REENT->_atexit;
> !   if (p->_ind >= _ATEXIT_SIZE)
> !     return -1;
> !   args = p->_on_exit_args_ptr;
> !   if (args == NULL)
> !     {
> !       args = malloc (sizeof * p->_on_exit_args_ptr);
> !       if (args == NULL)
> ! 	return -1;
> !       args->_fntypes = 0;
> !       p->_on_exit_args_ptr = args;
> !     }
> ! #else
>     if ((p = _REENT->_atexit) == NULL)
>       _REENT->_atexit = p = &_REENT->_atexit0;
>     if (p->_ind >= _ATEXIT_SIZE)
> *************** _DEFUN (on_exit,
> *** 80,96 ****
>         if ((p = (struct _atexit *) malloc (sizeof *p)) == NULL)
>   	return -1;
>         p->_ind = 0;
> !       p->_fntypes = 0;
>         p->_next = _REENT->_atexit;
>         _REENT->_atexit = p;
>       }
> ! #else
> !   p = &_REENT->_atexit;
> !   if (p->_ind >= _ATEXIT_SIZE)
> !     return -1;
>   #endif
> !   p->_fntypes |= (1 << p->_ind);
> !   p->_fnargs[p->_ind] = arg;
>     p->_fns[p->_ind++] = x;
>     return 0;
>   }
> --- 94,107 ----
>         if ((p = (struct _atexit *) malloc (sizeof *p)) == NULL)
>   	return -1;
>         p->_ind = 0;
> !       p->_on_exit_args._fntypes = 0;
>         p->_next = _REENT->_atexit;
>         _REENT->_atexit = p;
>       }
> !   args = & p->_on_exit_args;
>   #endif
> !   args->_fntypes |= (1 << p->_ind);
> !   args->_fnargs[p->_ind] = arg;
>     p->_fns[p->_ind++] = x;
>     return 0;
>   }
> 
> Index: newlib/libc/stdlib/exit.c
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/stdlib/exit.c,v
> retrieving revision 1.3
> diff -c -3 -p -r1.3 exit.c
> *** newlib/libc/stdlib/exit.c	15 May 2002 22:58:10 -0000	1.3
> --- newlib/libc/stdlib/exit.c	5 Jun 2003 09:28:36 -0000
> *************** _DEFUN (exit, (code),
> *** 60,79 ****
>   	int code)
>   {
>     register struct _atexit *p;
>     register int n;
> !   int i = 1;
>   
>   #ifdef _REENT_SMALL
> !   for (p = &_REENT->_atexit, n = p->_ind-1, i = (n>=0) ? (1<<n) : 0; 
> !        n >= 0; --n, i >>= 1)
>   #else
> !   for (p = _REENT->_atexit; p; p = p->_next)
> !     for (n = p->_ind - 1, i = (n >= 0) ? (1 << n) : 0; n >= 0; --n, i >>= 1)
>   #endif
> -       if (p->_fntypes & i)
> -         (*((void (*)(int, void *))p->_fns[n]))(code, p->_fnargs[n]);
> -       else
> -         (*p->_fns[n]) ();
>   
>     if (_REENT->__cleanup)
>       (*_REENT->__cleanup) (_REENT);
> --- 60,104 ----
>   	int code)
>   {
>     register struct _atexit *p;
> +   register struct _on_exit_args * args;
>     register int n;
> !   int i;
> ! 
> !   p = &_REENT->_atexit;
> ! 
> ! #define WALK_FUNCS       for (n = p->_ind - 1, i = (n >= 0) ? (1 << n) : 0; n >= 0; --n, i >>= 1)
>   
>   #ifdef _REENT_SMALL
> !   args = p->_on_exit_args_ptr;
> !   
> !   if (args == NULL)
> !     {
> !       WALK_FUNCS
> ! 	p->_fns[n] ();
> !     }
> !   else
> !     {
> !       WALK_FUNCS
> ! 	if (args->_fntypes & i)
> ! 	  (*((void (*)(int, void *)) p->_fns[n]))(code, args->_fnargs[n]);
> ! 	else
> ! 	  p->_fns[n] ();
> !     }
>   #else
> !   do
> !     {
> !       args = & p->_on_exit_args;
> !   
> !       WALK_FUNCS
> ! 	if (args->_fntypes & i)
> ! 	  (*((void (*)(int, void *)) p->_fns[n]))(code, args->_fnargs[n]);
> ! 	else
> ! 	  p->_fns[n] ();
> ! 
> !       p = p->_next;
> !     }
> !   while (p);
>   #endif
>   
>     if (_REENT->__cleanup)
>       (*_REENT->__cleanup) (_REENT);
> 




More information about the Newlib mailing list