Ivan Zhakov wrote: > Julian Foad 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.
OK, good. Thanks for explaining your opinion. I basically agree with you. (I don't care about exactly how they are combined, in detail.) When you wrote "Thanks for pointing to svn_string_from_stream(), but [it's different]" I (wrongly) thought you were implying that these differences made my argument invalid. That's where I misunderstood you. - Julian
