[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1555716 -/subversion/trunk/subversion/libsvn_ra_serf/update.c

From: Ivan Zhakov <ivan_at_visualsvn.com>
Date: Thu, 9 Jan 2014 13:18:24 +0400

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?

-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com
Received on 2014-01-09 10:19:23 CET

This is an archived mail posted to the Subversion Dev mailing list.