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 Wed, 07 Oct 2015 14:00:41 GMT
On 7 October 2015 at 16:50, Julian Foad <julianfoad@gmail.com> wrote:
> Ivan Zhakov wrote:
>> 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.
>
> Yes, it is slightly different. That doesn't justify it having a
> completely different implementation, does it? The length hint in
> _stringbuf... is optional, so it also has to operate without a length
> hint like _string_ does. The lack of a separate scratch pool is not a
> meaningful semantic difference.
>
> I would also add: why should they have slightly different API forms?
> They should not. Stupid differences like this just make things harder
> for no good reason. Let's make them identical, deprecating and bumping
> one or both of them to do so.
>
> It seems obvious to me that these functions are basically intended to
> do the same job, just with different output data type. Do you
> disagree? And if that is so, then they should have a common
> implementation. Do you disagree?
>

It looks like some misunderstanding between us: I never told that we
should not merge them to one implementation. I'm just saying that this
is ortogonal: making svn_string_from_stream() wrapper around
svn_stringbuf_from_stream()  is one task. Making
svn_stringbuf_from_stream() work efficient in all cases is another. My
point that svn_stringbuf_from_stream() should work fine in all cases:
whether it was called through svn_string_from_stream() or directly.

My personal opinion that svn_string_from_stream() should just call
svn_stringbuf_from_stream() with result pool and morph result to
string_t.

-- 
Ivan Zhakov

Mime
View raw message