On 05/20/2013 12:33 PM, Johan Herland wrote:
> On Sun, May 19, 2013 at 10:27 PM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
>> This is the culmination of the last few commits. Since some callers
>> want to store refnames in the name field of object_array elements, but
>> we don't want those callers to assume that the refnames that they got
>> from for_each_ref() have infinite lifetime, the easiest thing to do is
>> have object_array make a copy of the names before writing them in the
>> entries, and to free the names for entries that are no longer in use.
>> This change fixes the problem, but has some disadvantages:
>>
>> * It requires extra copies to be made of strings that are already
>> copies, for example when the results of path_name(path, name) are
>> used as a name in revision.c:add_object(). This might be rare
>> enough that it can be ignored (though the original result of
>> path_name() would have to be freed, which this patch doesn't do so
>> there is a memory leak).
>>
>> * Many callers store the empty string ("") as the name; for example,
>> most of the entries created during a run of rev-list have "" as
>> their name. This means that lots of needless copies of "" are being
>> made. I think that the best solution to this problem would be to
>> store NULL rather than "" for such entries, but I haven't figured
>> out all of the places where the name is used.
>
> Use strbufs?
>
> No allocation (except for the strbuf object itself) is needed for
> empty strings, and string ownership and be transferred to and from it
> to prevent extra copies.
That would cost two extra size_t per object_array_entry. I have the
feeling that this structure is used often enough that the extra overhead
would be a disadvantage, but I'm not sure.
The obvious alternative would be to teach users to deal with NULL and
either add another constructor alternative that transfers string
ownership or *always* transfer string ownership and change the callers
to call xstrdup() if they don't already own the name string. I think I
will try that approach first.
Michael
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html