On 05/20/2013 12:33 PM, Johan Herland wrote: > On Sun, May 19, 2013 at 10:27 PM, Michael Haggerty <mhag...@alum.mit.edu> > 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 mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html