Re: [PATCH][BZ 15089] Fixes malloc_trim always trims for large padding

[OndÅej BÃlka <neleai at seznam dot cz>]

On Wed, Nov 20, 2013 at 10:28:16AM +0000, Will Newton wrote:
> On 20 November 2013 04:38, Fernando J V da Silva
> <fernandojvdasilva@gmail.com> wrote:
> > Hi all!
> >
> > I'm starting learning about glibc code and I've began trying to fix
> > the bug 15089. I've put some comments on Bugzilla and I wrote a patch
> > (attached) for fixing it, based on the solution proposed by Thiago
> > Ize' comment in Bugzilla as well
> > (https://sourceware.org/bugzilla/show_bug.cgi?id=15089).
> >
> > That solution really fixes the problem, but the systrim() code still
> > mixes some signed and unsigned operations to calculate the size used
> > to call sbrk(), which may be dangerous (and in fact caused this bug).
> > I think the most correct solution would be to refactor this function
> > for only using unsigned variables but, to be honest, I haven't tried
> > it as I'm new on that (may be someone could give me some advices about
> > it?).
> >
> > I would be glad if someone could have some time to comment it! :-)
> 
> Hi Fernando,
> 
> Thanks for the patch!
> 
> In general the ternary operator tends to be quite hard to read so in
> my opinion is best avoided outside of macros. The overflow case should
> be quite simple to handle with a return 0, e.g.:
> 
> if (top_area <= pad)
>   return 0;
> 
> Then we can remove the whole "extra > 0" conditional below as well.
> 
> Also you need a GNU changelog entry to accompany your change (include
> it in the commit message rather than as a patch to ChangeLog as it
> makes it easier to merge) which mentions the chnages made and the BZ
> number that is being closed (see ChangeLog for examples of this).
> 
> Finally, and perhaps the most difficult, I think this change may need
> a copyright assignment on file to be accepted (see
> https://sourceware.org/glibc/wiki/Contribution%20checklist).
>
Copyrigth assignment is not needed for this patch but it is better to get it in
advance.

You need copyrigth assignment once total lines that you contributed
exceed 15 lines, see

http://www.gnu.org/prep/maintain/maintain.html#Legally-Significant