On 9 January 2014 13:29, Branko Čibej <brane_at_wandisco.com> wrote:
> On 09.01.2014 10:18, Ivan Zhakov wrote:
>
> On 7 January 2014 18:29, Bert Huijben <bert_at_qqmail.nl> wrote:
>
> On 7 January 2014 12:42, Bert Huijben <bert_at_qqmail.nl> wrote:
>
> The current code works in both scenarios (tested via buffersizes of just a
> few bytes, and a debugger verifyimg the corner cases, etc). But I doubt it
> is worth all the trouble. The file is null until something is spilled and
> from that point onwards everything is in the file (until the spill is
> empty).
>
> Few bytes is bad test because memory isn't used at all in this case.
>
> I've tested and code doesn't work as expected. > I used 128, 256 but didn't
> get a problem. The same values might also hit a
> corner case in the spill buffer. Will look at it in a few minutes.
>
> Hi Bert.
>
> I see you already removed this optimization in r1556343. Thanks.
>
> But it seems there is another problem with using spillbuf for request
> bodies: create_update_report_body() callback can be called multiple
> times if request need to be resend because of authentication challenge
> or KeepAlive limit. In this case you will get empty request body on
> second invocation, because all spillbuf is already read.
>
> I see two options how to fix this:
> 1. Do not use spillbuf for request bodies. Use custom implementation
> based on stringbuf and temporary file if needed.
> 2. Implement svn_spillbuf__rewind() or something.
>
> What do you think?
>
>
> Definitely implement rewind, IMO.
>
I'm not sure, because as far I remember orignally spillbuf was
implemented with queue/circular buffer behavior. But then spillbuf was
extended for other use cases, so may be there is no problem with
svn_spillbuf__rewind().
--
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com
Received on 2014-01-09 10:39:42 CET