Re: [PATCH] malloc: Add missing arena lock in mallinfo [BZ #22408]

[Florian Weimer <fweimer at redhat dot com>]

On 11/15/2017 04:53 AM, DJ Delorie wrote:
> Siddhesh Poyarekar <siddhesh@gotplt.org> writes:
> > I don't see how you came to that conclusion, there is a fix in there
> > that brings heap->size into the lock scope of ar_ptr->mutex.  That is
> > one fix and the other fix is to then expand the heap traversal so that
> > all heaps are accounted for.  Florian's follow-up patches have correctly
> > made that split.
>
> Florian's new patches move a line of code in one, and immediately remove
> it in the second, just to satisfy a BZ.  I doubt the readers of the BZ
> care about anything beyond "it was fixed" 99% of the time.

Don't forget I also added a new statistics to make the split a bit more 
palatable.

> > They're two very different bugs and hence should have two different
> > patches to clearly indicate what changed.
>
> I don't feel too strongly about this because the code is the same in the
> end, but it seems like extra work with a weak justification.  These are
> the types of requirements that may put off a first-time contributor.

But you weren't doing that to avoid a decision on patch acceptance.  I 
think this is the key difference that matters.

I'm more concerned with the documentation request for protection keys, 
for example.  The manual did not document mprotect before, so I have to 
fix that first.  But mprotect is complicated, and now we are looking at 
documenting the personality function.  Who knows where this will end? 
And all this goes into the manual, which very few people read because 
the manual pages are more immediately accessible (for both technical and 
non-technical reasons).

And all this when it's not even clear if we would accept the protection 
key patch in the end.

Thanks,
Florian