Reclaim _REENT_MP_P5S in _reclaim_reent

  From: Chris Johns <chrisj@rtems.org>

The change fixes a memory leak in threads that clean up using
_reclaim_reent.

RTEMS: Closes #4967
---
 newlib/libc/reent/reent.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
  

On Nov 16 12:12, chrisj@rtems.org wrote:
> From: Chris Johns <chrisj@rtems.org>
> 
> The change fixes a memory leak in threads that clean up using
> _reclaim_reent.
> 
> RTEMS: Closes #4967
> ---
>  newlib/libc/reent/reent.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/newlib/libc/reent/reent.c b/newlib/libc/reent/reent.c
> index db80ca06e..6bb271b9a 100644
> --- a/newlib/libc/reent/reent.c
> +++ b/newlib/libc/reent/reent.c
> @@ -59,6 +59,17 @@ _reclaim_reent (struct _reent *ptr)
>  	}
>        if (_REENT_MP_RESULT(ptr))
>  	_free_r (ptr, _REENT_MP_RESULT(ptr));
> +      if (_REENT_MP_P5S(ptr))
> +        {
> +         struct _Bigint *thisone, *nextone;
> +         nextone = _REENT_MP_P5S(ptr);
> +         while (nextone)
> +           {
> +             thisone = nextone;
> +             nextone = nextone->_next;
> +             _free_r (ptr, thisone);
> +           }
> +       }

The p5s data is allocated using Balloc, so the pointers are in freelist
and should already be free'd at this point, starting at line 42.

Please explain what happens and why free'ing the freelist is not
sufficient in this case, preferredly as part of the commit message.


Thanks,
Corinna
  

On 16/11/2023 8:15 pm, Corinna Vinschen wrote:
> On Nov 16 12:12, chrisj@rtems.org wrote:
>> From: Chris Johns <chrisj@rtems.org>
>>
>> The change fixes a memory leak in threads that clean up using
>> _reclaim_reent.
>>
>> RTEMS: Closes #4967
>> ---
>>  newlib/libc/reent/reent.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/newlib/libc/reent/reent.c b/newlib/libc/reent/reent.c
>> index db80ca06e..6bb271b9a 100644
>> --- a/newlib/libc/reent/reent.c
>> +++ b/newlib/libc/reent/reent.c
>> @@ -59,6 +59,17 @@ _reclaim_reent (struct _reent *ptr)
>>  	}
>>        if (_REENT_MP_RESULT(ptr))
>>  	_free_r (ptr, _REENT_MP_RESULT(ptr));
>> +      if (_REENT_MP_P5S(ptr))
>> +        {
>> +         struct _Bigint *thisone, *nextone;
>> +         nextone = _REENT_MP_P5S(ptr);
>> +         while (nextone)
>> +           {
>> +             thisone = nextone;
>> +             nextone = nextone->_next;
>> +             _free_r (ptr, thisone);
>> +           }
>> +       }
> 
> The p5s data is allocated using Balloc, so the pointers are in freelist
> and should already be free'd at this point, starting at line 42.

Yes however P5S blocks allocated by Balloc via i2b are not passed to Bfree.

> Please explain what happens and why free'ing the freelist is not
> sufficient in this case, preferredly as part of the commit message.

Yes good idea. How about:

The _REENT_MP_P5S blocks are allocated using Balloc via i2b and linked in the
pow5mult call. As a result these blocks are not on the freelist managed by the
Bfree call. This change fixes a memory leak in threads that clean up using
_reclaim_reent.

Chris
  

On Nov 17 08:17, Chris Johns wrote:
> On 16/11/2023 8:15 pm, Corinna Vinschen wrote:
> > On Nov 16 12:12, chrisj@rtems.org wrote:
> >> From: Chris Johns <chrisj@rtems.org>
> >>
> >> The change fixes a memory leak in threads that clean up using
> >> _reclaim_reent.
> >>
> >> RTEMS: Closes #4967
> >> ---
> >>  newlib/libc/reent/reent.c | 11 +++++++++++
> >>  1 file changed, 11 insertions(+)
> >>
> >> diff --git a/newlib/libc/reent/reent.c b/newlib/libc/reent/reent.c
> >> index db80ca06e..6bb271b9a 100644
> >> --- a/newlib/libc/reent/reent.c
> >> +++ b/newlib/libc/reent/reent.c
> >> @@ -59,6 +59,17 @@ _reclaim_reent (struct _reent *ptr)
> >>  	}
> >>        if (_REENT_MP_RESULT(ptr))
> >>  	_free_r (ptr, _REENT_MP_RESULT(ptr));
> >> +      if (_REENT_MP_P5S(ptr))
> >> +        {
> >> +         struct _Bigint *thisone, *nextone;
> >> +         nextone = _REENT_MP_P5S(ptr);
> >> +         while (nextone)
> >> +           {
> >> +             thisone = nextone;
> >> +             nextone = nextone->_next;
> >> +             _free_r (ptr, thisone);
> >> +           }
> >> +       }
> > 
> > The p5s data is allocated using Balloc, so the pointers are in freelist
> > and should already be free'd at this point, starting at line 42.
> 
> Yes however P5S blocks allocated by Balloc via i2b are not passed to Bfree.

Aaah, right, it really never does.

> > Please explain what happens and why free'ing the freelist is not
> > sufficient in this case, preferredly as part of the commit message.
> 
> Yes good idea. How about:
> 
> The _REENT_MP_P5S blocks are allocated using Balloc via i2b and linked in the
> pow5mult call. As a result these blocks are not on the freelist managed by the
> Bfree call. This change fixes a memory leak in threads that clean up using
> _reclaim_reent.

Yeah, sounds good.  Please resend with this commit message.


Thanks,
Corinna