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