On 7 October 2015 at 16:21, Julian Foad <julianf...@gmail.com> wrote: >>>> Julian Foad wrote: >>>>> [...] I will be >>>>> interested in reviewing the (single) implementation. > > Stefan wrote: >> [...] Here the final version. >> If that doesn't work either then I'm done for today. > > Stefan: First review comment: You can much more efficiently convert a > stringbuf to a string (when you own it, like here) using > svn_stringbuf__morph_into_string(). Second review question: did you > consider whether the second implementation was in fact already better? > For example, is using _appendbytes better or worse than using > ensure(size x 2) followed by read()? >
> Ivan wrote: >> Merging svn_stringbuf_from_stream() and svn_stringbuf_from_stream() >> implementations is completely ortogonal from my point of view. >> >> But I really don't understand what was your point in r1707196 review >> if you are not interested in review any changes to this function at >> all. > > Ivan: I reviewed your commit because I saw it. During review I *then* > thought "surely we must already have some code for doing this" and so > I went looking for similar code and found the existing similar > function. And combining them is not orthogonal to tweaking one of them > on its own. (Combining them would be orthogonal to tweaking both of > the implementations together, if the implementations were initially > similar enough so that would make sense.) Tweaking one of them on its > own doesn't give me any clue whether you have considered whether the > other implementation might already be better than what you're doing. > Thanks for pointing to svn_string_from_stream(), but this function slightly different: it has SCRATCH_POOL argument and doesn't have LEN_HINT argument. It's little difference in semantic. -- Ivan Zhakov
