On 23 October 2015 at 13:13, Julian Foad <julianfoad_at_gmail.com> wrote:
> On 22 October 2015, Ivan Zhakov wrote:
>> On 19 October 2015, Ivan Zhakov wrote:
>>> [[[
>>> Revv svn_string_from_stream() function and share implementation with
>>> svn_stringbuf_from_stream().
>>>
>>> Suggested by: julianf
> [...]
>>> ]]]
>>>
>> Committed in r1710065.
>>
>>> [[[
>>> * subversion/libsvn_subr/stream.c
>>> (svn_stringbuf_from_stream): Optimize memory usage a bit and avoid
>>> svn_stream_read_full() call once we got partial read.
>>> ]]]
>>>
>> Committed in r1710066.
>
> OK, looks good. (I haven't reviewed the implementation in detail, yet.)
>
> Two more things:
>
> 1. The doc string should explain len_hint in the caller's terms. The
> doc strings also differ in other unnecessary respects. I propose the
> attached documentation patch.
>
I agree. Patch look great!
> 2. The _string and _stringbuf function declarations still differ in
> that only svn_string_from_stream2() takes a scratch pool. The current
> implementation doesn't use it. We should unify them. Which is best,
> taking a scratch pool or not?
>
> Stefan Fuhrmann wrote earlier:
>> Maybe get rid of the scratch_pool because we won't use it nor do any of the current
>> callers care.
>
> In the absence of any other argument, I would accept that argument and
> remove the scratch pool.
>
I don't have opinion on this, so feel free to remove scratch_pool
argument if you think that it will be useful.
--
Ivan Zhakov
Received on 2015-10-23 12:42:27 CEST