subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan Zhakov <i...@visualsvn.com>
Subject Re: svn commit: r1707196 - /subversion/trunk/subversion/libsvn_subr/stream.c
Date Fri, 23 Oct 2015 10:42:03 GMT
On 23 October 2015 at 13:13, Julian Foad <julianfoad@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

Mime
View raw message