This is the mail archive of the 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] Fix potential reent issue

On 06/24/2013 05:37 AM, Corinna Vinschen wrote:
On Jun 23 22:13, Freddie Chopin wrote:
W dniu 2013-06-23 21:40, Federico Terraneo pisze:
On 06/23/2013 07:53 PM, Freddie Chopin wrote:

Wouldn't it be cleaner/simpler to use "_REENT" symbol (which is
either "_impure_ptr" or "__getreent()") in place of faulty uses of

The problem is that when _REENT_ONLY is defined, _REENT does not get
defined (look at sys/reent.h). I think it's this fact that caused the
faulty use of _impure_ptr, but if the definition of _REENT is enclosed
in an #ifndef _REENT_ONLY, there's probably a reason.

Yes, you're right...

Looking at repo history it seems that the _impure_ptr solution was
older, then - 11 years ago - the one using _REENT was added, but the
old variant remained unchanged for years.

It seems to me that the "right" solution for _REENT_ONLY build would
be to undefine stdin/stdout/stderr - delete the offending lines
using _impure_ptr...

If that solution is not feasible then your approach is probably the
best one.

BTW - I've CCed the list, you probably forgot about that.

I leave this problem for Jeff.

Let me first state that historically the _REENT_ONLY solution was initially added long ago for those who wanted to support multi-threading and thus had to keep track of separate reentrancy pointers. The macro helped find usage of non _r functions. I do not believe the std stream macros were ever restricted as part of this. You either defaulted to the global reent struct or used the _r versions.

The addition of _DYNAMIC_REENT obsoleted the need for _REENT_ONLY because the __getreent() function would take care of finding the right reentrancy structure for all calls and the code did not need to use _r functions at all.

I do not think it is meaningful for anyone to use both macros at the same time. If you choose to use _REENT_ONLY as well, then you are unnecessarily forcing the user to recode their application to only work under newlib.

Undefining the std stream macros now would be API removal that could break existing code and for no real good reason.

Regarding your patch, a better solution IMO would be to remove the _REENT_ONLY check in sys/reent.h so that the _REENT macro was defined and thus the std streams could always be defined in terms of _REENT. It ends up the same and makes the header files cleaner.

I have attached the patch.

-- Jeff J.

Index: stdio.h
RCS file: /cvs/src/src/newlib/libc/include/stdio.h,v
retrieving revision 1.63
diff -p -r1.63 stdio.h
*** stdio.h	1 Nov 2012 11:51:11 -0000	1.63
--- stdio.h	24 Jun 2013 18:49:43 -0000
*************** typedef _fpos64_t fpos64_t;
*** 139,153 ****
  #define	TMP_MAX		26
- #ifndef _REENT_ONLY
  #define	stdin	(_REENT->_stdin)
  #define	stdout	(_REENT->_stdout)
  #define	stderr	(_REENT->_stderr)
- #else /* _REENT_ONLY */
- #define	stdin	(_impure_ptr->_stdin)
- #define	stdout	(_impure_ptr->_stdout)
- #define	stderr	(_impure_ptr->_stderr)
- #endif /* _REENT_ONLY */
  #define _stdin_r(x)	((x)->_stdin)
  #define _stdout_r(x)	((x)->_stdout)
--- 139,147 ----
Index: wchar.h
RCS file: /cvs/src/src/newlib/libc/include/wchar.h,v
retrieving revision 1.30
diff -p -r1.30 wchar.h
*** wchar.h	1 Nov 2012 11:51:11 -0000	1.30
--- wchar.h	24 Jun 2013 18:49:43 -0000
*************** int	_EXFUN(_wscanf_r, (struct _reent *, 
*** 179,191 ****
  #define getwc(fp)	fgetwc(fp)
  #define putwc(wc,fp)	fputwc((wc), (fp))
- #ifndef _REENT_ONLY
  #define getwchar()	fgetwc(_REENT->_stdin)
  #define putwchar(wc)	fputwc((wc), _REENT->_stdout)
- #else
- #define getwchar()	fgetwc(_impure_ptr->_stdin)
- #define putwchar(wc)	fputwc((wc), _impure_ptr->_stdout)
- #endif
--- 179,186 ----
Index: sys/reent.h
RCS file: /cvs/src/src/newlib/libc/include/sys/reent.h,v
retrieving revision 1.53
diff -p -r1.53 reent.h
*** sys/reent.h	8 May 2013 23:13:51 -0000	1.53
--- sys/reent.h	24 Jun 2013 18:49:43 -0000
*************** void _reclaim_reent _PARAMS ((struct _re
*** 797,804 ****
  /* #define _REENT_ONLY define this to get only reentrant routines */
- #ifndef _REENT_ONLY
  #if defined(__DYNAMIC_REENT__) && !defined(__SINGLE_THREAD__)
  #ifndef __getreent
    struct _reent * _EXFUN(__getreent, (void));
--- 797,802 ----
*************** void _reclaim_reent _PARAMS ((struct _re
*** 808,815 ****
  # define _REENT _impure_ptr
  #endif /* __SINGLE_THREAD__ || !__DYNAMIC_REENT__ */
- #endif /* !_REENT_ONLY */
  #define _GLOBAL_REENT _global_impure_ptr
--- 806,811 ----

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]