On 9 January 2014 13:29, Branko Čibej <br...@wandisco.com> wrote: > On 09.01.2014 10:18, Ivan Zhakov wrote: > > On 7 January 2014 18:29, Bert Huijben <b...@qqmail.nl> wrote: > > On 7 January 2014 12:42, Bert Huijben <b...@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