C. Michael Pilato wrote:
> Having trouble typing clearly today, it seems...
>
> C. Michael Pilato wrote:
>> Hyrum K. Wright wrote:
>>
>>> This patch fixes a bug where multiple --target switches fail silently
>>> for all but the final one. Instead, this patch concatenates all targets
>>> specified by multiple --target switches into one array.
>>>
>>> Coincidentally, this patch also fixes a memory leak, since we were just
>>> dropping the previous list of targets when we encountered additional
>>> --target switches.
>>
>> Eh, we're still leaking memory with this patch because apr_array_append()
>> doesn't reuse either of the arrays it's given as input. So it takes array A
>> and B and makes a new C (wasting both A and B). If a third --targets comes
>> along, it'll waste another pair of arrays. And so on. But we're only
>> talking about an amount of memory proportional to the number of times
>> --targets shows up as an option, and not alot of memory at that.
>
> To clarify, it only wastes the memory needed for the array structure, not
> its elements.
Understood. Is this type of waste acceptable, or would the ideal
solution be to make sure we clean up after ourselves by using a subpool?
>> Still, if you know you're going to be wasting memory anyway, you can
>> simplify the code like so:
>>
>> if (! opt_state.targets)
>> opt_state.targets = apr_array_make(pool, 8, sizeof(const char *));
>> opt_state.targets = apr_array_append(pool, opt_state.targets,
>> svn_cstring_split(buffer_utf8->data,
>> "\n\r", TRUE,
>> pool));
>
> And (duh), no need to allocate a spot for 8 members on the wasted array above.
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed May 31 23:19:52 2006