On 23 October 2015 at 13:13, Julian Foad <julianf...@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
