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

Re: svn commit: r1555774 - in /subversion/trunk/subversion/libsvn_ra_serf: eagain_bucket.c ra_serf.h update.c util.c

From: Ivan Zhakov <ivan_at_visualsvn.com>
Date: Tue, 7 Jan 2014 02:54:29 +0400

On 6 January 2014 17:59, <rhuijben_at_apache.org> wrote:
> Author: rhuijben
> Date: Mon Jan 6 13:59:45 2014
> New Revision: 1555774
>
> URL: http://svn.apache.org/r1555774
> Log:
> In libsvn_ra_serf, replace the pause handling for the update response that was
> embedded through the old -not transition based- xml parser with a pause
> handling response handler wrapper directly in the update logic.
>
Hi Bert, see my comments below.

> +
> +static apr_status_t
> +eagain_bucket_read(serf_bucket_t *bucket,
> + apr_size_t requested,
> + const char **data,
> + apr_size_t *len)
> +{
> + eagain_baton_t *eab = bucket->data;
> +
> + if (eab->remaining > 0)
> + {
> + *data = eab->data;
> + if (requested > eab->remaining || requested == SERF_READ_ALL_AVAIL)
> + {
> + *len = eab->remaining;
> + eab->data = NULL;
> + eab->remaining = 0;
> + }
> + else
> + {
> + *len = requested;
> + eab->data += requested;
> + eab->remaining -= requested;
> + }
> +
> + if (eab->remaining)
> + return APR_SUCCESS;
> + }
> +
> + return APR_EAGAIN;
> +}
> +
Alternative approach is reuse SIMPLE bucket at make eagain_bucket
wrapper around any inner bucket transforming APR_EOF to APR_EAGAIN.

[...]

> +
> +static apr_status_t
> +eagain_bucket_peek(serf_bucket_t *bucket,
> + const char **data,
> + apr_size_t *len)
> +{
> + const eagain_baton_t *eab = bucket->data;
> +
> + *data = eab->data;
> + *len = eab->remaining;
> +
> + return *data == NULL ? APR_EOF : APR_SUCCESS;
> +}
Are you sure about returning APR_EOF here?

[...]

>
> Modified: subversion/trunk/subversion/libsvn_ra_serf/update.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/update.c?rev=1555774&r1=1555773&r2=1555774&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_serf/update.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_serf/update.c Mon Jan 6 13:59:45 2014
[...]

> +/* Delaying wrapping reponse handler, to avoid creating too many
> + requests to deliver efficiently */
> +static svn_error_t *
> +update_delay_handler(serf_request_t *request,
> + serf_bucket_t *response,
> + void *handler_baton,
> + apr_pool_t *scratch_pool)
> +{
[...]
> + if (len == 0 && !at_eof)
> + return svn_ra_serf__wrap_err(status, NULL);
> +
> + /* If not at EOF create a bucket that finishes with EAGAIN, otherwise
> + use a standard bucket with default EOF handling */
> + err = udb->inner_handler(request,
> + at_eof
> + ? serf_bucket_simple_create(
> + data, len, NULL, NULL,
> + serf_request_get_alloc(request))
> + : svn_ra_serf__create_bucket_with_eagain(
> + data, len,
> + serf_request_get_alloc(request)),
With my suggestion above this code will be converted to something like:
[[[
bucket = serf_bucket_simple_create();
if (!at_eof)
  bucket = svn_ra_serf__create_bucket_with_eagain(bucket);
err = udb->inner_handler(request, bucket, udb->inner_handler_baton, iterpool);
]]]

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

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.