Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:

> On Tue, Dec 2, 2014 at 7:11 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
>> On Tue, Dec 2, 2014 at 2:32 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>> Nguyễn Thái Ngọc Duy  <pclouds@xxxxxxxxx> writes:
>>>
>>>> This allows the callback to use 'base' as a temporary buffer to
>>>> quickly assemble full path "without" extra allocation. The callback
>>>> has to restore it afterwards of course.
>>>
>>> Hmph, what's the quote around 'without' doing there?
>>
>> because it's only true if you haven't used up all preallocated space
>> in strbuf. If someone passes an empty strbuf, then underneath strbuf
>> may do a few realloc until the buffer is large enough.
>
> Would it be easier to understand if written like this?
>
>     This allows the callback to use 'base' as a temporary buffer to
>     quickly assemble full path, thus avoiding allocation/deallocation
>     for each iteration step.

Thanks.

I am not Duy so I do not know if that matches what he wanted to say,
or if it matches what the change gives us.  Your phrasing wouldn't
have made me say "Hmph...?" and it is an improvement that goes in
the right direction, I think.

A question during the review, especially on proposed log messages
and documentation changes, is rarely just a request to explain it to
the questioner in the discussion. It is an indication that what is
being commented on needs tweaking to be understood.
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]