* Carlos O'Donell:
On 6/24/19 7:11 AM, Florian Weimer wrote:
> * Carlos O'Donell:
>
>> I am fairly confident that the code works and doesn't cause serious
>> problems, but I'd like a double check from both of you.
>
> The new algorithm will not find a chunk if it has been consolidated with
> its preceding chunk (at a lower address), but not with its successor.
This is true.
We could improve it to try look for this case.
I see 3 cases:
1- Consolidation has not happened, and we have the header, chunk, and
remainder, and chunk is aligned, we find it and we return.
2- Consolidation has happened and we have one large chunk again, and
it's unaligned, but we'll use it, and split it into a header, chunk
and remainder; returning the aligned chunk.
3- Consolidation has happened and we have consolidated only with the
header, and we have a perfect opportunity to split just the header
off.
We handle case 1 and 2, but not 3.
Did I describe your position correctly?
I failed to mention that consolidation with its successor moves a chunk
to a different size bin. We won't find that either.
> The unsorted bins aren't searched, either.
This is a potentially costly operation.
I don't think so, regular malloc has to do it as well, so at worst we
search it twice, but it could also search it once, with a better search
criterion.
> I expected that you would search bins which are larger than the
request
> size, too, in case the aligned pointer is further into the allocation.
This is equivalent to case 3 above, you can think of it as a case where
consolidation occurred, and we might want to split into a header + chunk.
> The list updates lack the usual security checks, and there are some
> implicit null pointer checks (style-only issue).
We can clean these up with some subsequent refinements.
So let me summarize:
a- We support only the additional case 1, which is sufficiently sized and
aligned chunks.
Not sufficiently sized, exactly sized.
My concern is that it's very hard to describe what this change does.
We can't release F31 with this patch so we will have to back it
out before
too long. Is this a problem given the imminent release? Do we need to wait
for F31 to branch before doing this kind of testing?
Ideally, we would wait, yes.
Thanks,
Florian