Re: svn commit: r1694533 - /subversion/trunk/subversion/libsvn_ra_svn/marshal.c

On 25.08.2015 23:08, Stefan Fuhrmann wrote:
>> All right, so I figured that the difference is that apr_array_make does
>> two allocations compared to one in this code. Still: relying on
>> knowledge of APR implementation details is a really bad idea.
>
> The structure definition of apr_array_header_t is part of
> the public API, i.e. will never change.

The semantics might, even if the shape of the structure itself doesn't.

>> As it
>> stands, APR will correctly resize this array if necessary; but there's
>> no guarantee that, sometime in the future, resizing such arrays would
>> fail horribly.
>>
> Even if that was true, the resize is done outside APR as
> well - a few lines further down.

Indeed it is ... and that code is essentially a copy-paste from
apr_tables.c.

I still think this kind of performance hack belongs in APR. Users that
don't have a new-enough APR won't get the performance boost, but on the
other hand, the kind of bug that started this discussion will stay out
of our code.

I think we've had our fair share of alignment bugs with all the
hand-crafted allocations etc.; we may as well stop now.

-- Brane
Received on 2015-08-26 10:18:55 CEST