Re: [PATCH] Delete default __getreent() implementation

[Craig Howland <howland at LGSInnovations dot com>]

On 11/07/2017 10:03 AM, Corinna Vinschen wrote:
> On Nov  7 12:53, Sebastian Huber wrote:
> > Systems defining  __DYNAMIC_REENT__ have to provide a proper
> > implementation of __getreent().  Do not include a default implementation
> > to avoid a potential link time issue (real vs. default implementation of
> > __getreent()).
> Ok, please push (without the kill_r removal).
>
>
> Thanks,
> Corinna
Hold on a minute!  This patch is making assumptions that are not necessarily 
true, and will break backwards compatibility.  Quoting from reent.h:

 "Dynamic reentrancy is specified by the __DYNAMIC_REENT__ flag. This
   flag denotes setting up a macro to replace _REENT with a function call
   to __getreent().  This function needs to be implemented by the platform
   and it is meant to return the reentrancy structure for the current
   thread."

While this text has an implied expectation that the platform is intended to 
supply the function, this implication is not true because there is a default 
implementation--one that really does work.  And given it is there by default, 
people have likely just used it.  In fact, I have an implementation from 10 
years ago that does use the default implementation.  (The task switch is very 
simple, just re-assigning the value of impure_ptr.)  While I do not presently 
have plans to change from the old Newlib used in that project, it is an obvious 
example of breakage if I were to need to update that application to a Newlib 
version in which this change had been made.  It is easy to envision that others 
have done the same thing.

I suggest that at a minimum the contents of getreent.c should be kept.  This 
keeps a starting point for people who are needing to supply such a function.  
(Leaving it out of the library stops the link problem which is the stated goal 
of the patch.)  But I think it needs to go further, and this patch should not be 
done in this form because of the cited backwards-compatibility reasons.  If 
something were to be done it should be a configure flag to leave it out of the 
library.

Craig