Evgeny Kotkov wrote on Fri, Jul 28, 2017 at 15:17:24 +0300:
> Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:
>
> > I did misunderstand the relation between the two streams, …
> >
> >> The internal buffer doesn't grow beyond the (header size + max window
> >> size in svndiff format), which is O(1).
> >
> > … but I still don't see why this statement is true. The stream is
> > written to by [a callback provided by] svn_txdelta_to_svndiff3(), and
> > that function does not promise to write into the stream one svndiff
> > window per call. The API contract permits it to write into the stream
> > more than one svndiff window per call, in which case the buffer's size
> > will not be O(1).
>
> I don't see any real issues with relying on the implementation detail (how
> the window_handler() function works) of the API from the same module.
>
On the one hand, it _is_ the same module, but on the other hand,
svn_txdelta_to_svndiff_stream() does not call any private functions.
That gave me the impression it was meant to be a black box use of the
svn_txdelta_to_svndiff3() API.
> Perhaps, I could add a couple of comments on this.
>
That would be great. It'll ensure we don't forget about this dependency
when we revv one of these functions.
> To avoid this kind of coupling altogether, we could implement the alternative
> approach from my previous e-mail. Which is, provide a read handler that
> emits the svndiff header, calls encode_window() per every txdelta window
> and keeps the part of the encoded window that the caller can't accept at
> this time, thus postponing it to the next call of svn_stream_read(). But, in
> the meanwhile, I don't think that such implementation is currently required.
I'm not sure I fully see this alternative, but in any case, I'm mainly
concerned here about fulfilling the various API contracts. Anything
that does so is fine by me. (Or we could just document the reliance on
unpromised behaviour, as you suggested above)
Cheers,
Daniel
Received on 2017-07-31 17:21:21 CEST