On 7 October 2015 at 16:21, Julian Foad <julianfoad_at_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
Received on 2015-10-07 15:31:31 CEST