Subject: Re: [PATCH 2/7] Simplify strbuf uses in archive-tar.c using the proper functions.

From: René Scharfe <[email-masked]>
Date: Fri, 07 Sep 2007 00:54:39 +0200

Pierre Habouzit schrieb:
> On Thu, Sep 06, 2007 at 06:18:34PM +0000, Kristian Høgsberg wrote:
>> On Thu, 2007-09-06 at 20:08 +0200, Pierre Habouzit wrote:
>>> On Thu, Sep 06, 2007 at 05:59:29PM +0000, Kristian Høgsberg wrote:
>>>> On Thu, 2007-09-06 at 13:20 +0200, Pierre Habouzit wrote:
>>>>> -	memcpy(path.buf, base, baselen);
>>>>> -	memcpy(path.buf + baselen, filename, filenamelen);
>>>>> -	path.len = baselen + filenamelen;
>>>>> -	path.buf[path.len] = '\0';
>>>>> +	strbuf_grow(&path, MAX(PATH_MAX, baselen + filenamelen + 1));
>>>>> +	strbuf_reset(&path);
>>>> Does strbuf_reset() do anything here?
>>>>
>>>>> +	strbuf_add(&path, base, baselen);
>>>   Yes _reset() sets length to 0. so the add here will write at the start
>>> of the buffer again. It definitely is important !
>> But where was length set to non-zero?  path is initialized on entry to
>> the function, and strbuf_grow() just increases the allocation, not
>> length, right?
> 
>   The path is static, hence when you reenter the function you have the
> last value in it. The fact that it's static may be questionable, but it
> was like it before, I kept it, I've supposed it was for performance
> reasons.

You're right, I've made it static to get the number of mallocs from
number-of-files-in-archive down to one.  It was a ~10% speedup in the
warm cache case, so I could not resist. :)

René