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