Re: svn commit: r22514 - trunk/subversion/libsvn_ra_dav

Would it be possible to use a baton wrapper instead to pass to
svn_ra_dav__xml_collect_cdata()? The API would be cleaner.

On Wed, 29 Nov 2006, dionisos@tigris.org wrote:

> Author: dionisos
> Date: Wed Nov 29 23:20:24 2006
> New Revision: 22514
>
> Log:
> Migrate more parsers to post Neon 0.23 parser interface, eliminating
> manual tracking of errors where possible.
>
> Note: No real compatibility is lost here:
> we haven't supported Neon 0.23 for a looong time.
>
>
> * subversion/libsvn_ra_dav/ra_dav.h
> * subversion/libsvn_ra_dav/util.c
> (svn_ra_dav__xml_collect_cdata): New. Collects cdata into a stringbuf.
>
> * subversion/libsvn_ra_dav/merge.c
> (start_element): Adjust to new interface, making sure we only collect
> required cdata.
> (end_element): Adjust to new interface.
> (merge_ctx_t): Add fields to collect cdata into. Remove 'err' field, as
> we no longer need to manually track errors.
> (svn_ra_dav__merge_activity): Stop tracking errors manually. Also adjust
> for new parser interface.
>
> * subversion/libsvn_ra_dav/fetch.c
> (drev_baton_t): New. We need a place to store collected cdata.
> (drev_start_element,
> drev_end_element): Adjust to new interface and start using baton.
> (svn_ra_dav__get_dated_revision): Initialize baton and adjust for new
> parser interface.
>
> * subversion/libsvn_ra_dav/options.c
> (options_ctx_t): Add fields to collect cdata into.
> (validate_element): No longer part of the parser interface; remove
> unused parameter.
> (start_element): Adjust to new interface. Also make sure we collect
> only wanted cdata.
> (end_element): Adjust to new interface.
> (svn_ra_dav__get_activity_collection): Adjust to new parser interface.
>
>
>
> Modified:
> trunk/subversion/libsvn_ra_dav/fetch.c
> trunk/subversion/libsvn_ra_dav/merge.c
> trunk/subversion/libsvn_ra_dav/options.c
> trunk/subversion/libsvn_ra_dav/ra_dav.h
> trunk/subversion/libsvn_ra_dav/util.c
>
> Modified: trunk/subversion/libsvn_ra_dav/fetch.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_dav/fetch.c?pathrev=22514&r1=22513&r2=22514
> ==============================================================================
> --- trunk/subversion/libsvn_ra_dav/fetch.c (original)
> +++ trunk/subversion/libsvn_ra_dav/fetch.c Wed Nov 29 23:20:24 2006
> @@ -1181,26 +1181,45 @@
> ** the end-element handler for DAV:version-name.
> */
>
> -/* This implements the `svn_ra_dav__xml_validate_cb' prototype. */
> -static int drev_validate_element(void *userdata, svn_ra_dav__xml_elmid parent,
> - svn_ra_dav__xml_elmid child)
> +typedef struct
> {
> - return SVN_RA_DAV__XML_VALID;
> -}
> + svn_stringbuf_t *cdata;
> + apr_pool_t *pool;
> + svn_revnum_t revision;
> +} drev_baton_t;
> +
>
> /* This implements the `svn_ra_dav__xml_startelm_cb' prototype. */
> -static int drev_start_element(void *userdata, const svn_ra_dav__xml_elm_t *elm,
> - const char **atts)
> +static svn_error_t *
> +drev_start_element(int *elem, void *baton, int parent,
> + const char *nspace, const char *name, const char **atts)
> {
> - return SVN_RA_DAV__XML_VALID;
> + const svn_ra_dav__xml_elm_t *elm =
> + svn_ra_dav__lookup_xml_elem(drev_report_elements, nspace, name);
> + drev_baton_t *b = baton;
> +
> + *elem = elm ? elm->id : SVN_RA_DAV__XML_DECLINE;
> + if (!elm)
> + return SVN_NO_ERROR;
> +
> + if (elm->id == ELEM_version_name)
> + b->cdata = svn_stringbuf_create("", b->pool);
> +
> + return SVN_NO_ERROR;
> }
>
> /* This implements the `svn_ra_dav__xml_endelm_cb' prototype. */
> -static int drev_end_element(void *userdata, const svn_ra_dav__xml_elm_t *elm,
> - const char *cdata)
> +static svn_error_t *
> +drev_end_element(void *baton, int state,
> + const char *nspace, const char *name)
> {
> - if (elm->id == ELEM_version_name)
> - *((svn_revnum_t *) userdata) = SVN_STR_TO_REV(cdata);
> + drev_baton_t *b = baton;
> +
> + if (state == ELEM_version_name && b->cdata)
> + {
> + b->revision = SVN_STR_TO_REV(b->cdata->data);
> + b->cdata = NULL;
> + }
>
> return SVN_RA_DAV__XML_VALID;
> }
> @@ -1214,11 +1233,15 @@
> const char *body;
> const char *vcc_url;
> svn_error_t *err;
> + drev_baton_t *b = apr_palloc(pool, sizeof(*b));
> +
> + b->pool = pool;
> + b->cdata = NULL;
> + b->revision = SVN_INVALID_REVNUM;
>
> /* Run the 'dated-rev-report' on the VCC url, which is always
> guaranteed to exist. */
> SVN_ERR(svn_ra_dav__get_vcc(&vcc_url, ras->sess, ras->root.path, pool));
> -
>
> body = apr_psprintf(pool,
> "<?xml version=\"1.0\" encoding=\"utf-8\"?>"
> @@ -1228,23 +1251,23 @@
> "</S:dated-rev-report>",
> svn_time_to_cstring(timestamp, pool));
>
> - *revision = SVN_INVALID_REVNUM;
> - err = svn_ra_dav__parsed_request_compat(ras->sess, "REPORT",
> - vcc_url, body, NULL, NULL,
> - drev_report_elements,
> - drev_validate_element,
> - drev_start_element, drev_end_element,
> - revision, NULL, NULL, FALSE, pool);
> + err = svn_ra_dav__parsed_request(ras->sess, "REPORT",
> + vcc_url, body, NULL, NULL,
> + drev_start_element,
> + svn_ra_dav__xml_collect_cdata,
> + drev_end_element,
> + b, NULL, NULL, FALSE, pool);
> if (err && err->apr_err == SVN_ERR_UNSUPPORTED_FEATURE)
> return svn_error_quick_wrap(err, _("Server does not support date-based "
> "operations"));
> else if (err)
> return err;
>
> - if (*revision == SVN_INVALID_REVNUM)
> + if (b->revision == SVN_INVALID_REVNUM)
> return svn_error_create(SVN_ERR_INCOMPLETE_DATA, NULL,
> _("Invalid server response to dated-rev request"));
>
> + *revision = b->revision;
> return SVN_NO_ERROR;
> }
>
>
> Modified: trunk/subversion/libsvn_ra_dav/merge.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_dav/merge.c?pathrev=22514&r1=22513&r2=22514
> ==============================================================================
> --- trunk/subversion/libsvn_ra_dav/merge.c (original)
> +++ trunk/subversion/libsvn_ra_dav/merge.c Wed Nov 29 23:20:24 2006
> @@ -2,7 +2,7 @@
> * merge.c : routines for performing a MERGE server requests
> *
> * ====================================================================
> - * Copyright (c) 2000-2005 CollabNet. All rights reserved.
> + * Copyright (c) 2000-2006 CollabNet. All rights reserved.
> *
> * This software is licensed as described in the file COPYING, which
> * you should have received as part of this distribution. The terms
> @@ -75,15 +75,17 @@
> };
>
> typedef struct {
> + /*WARNING: WANT_CDATA should stay the first element in the baton:
> + svn_ra_dav__xml_collect_cdata() assumes the baton starts with a stringbuf.
> + */
> + svn_stringbuf_t *want_cdata;
> + svn_stringbuf_t *cdata;
> apr_pool_t *pool;
>
> /* a clearable subpool of pool, for loops. Do not use for anything
> that must persist beyond the scope of your function! */
> apr_pool_t *scratchpool;
>
> - /* any error that may have occurred during the MERGE response handling */
> - svn_error_t *err;
> -
> /* the BASE_HREF contains the merge target. as resources are specified in
> the merge response, we make their URLs relative to this URL, thus giving
> us a path for use in the commit callbacks. */
> @@ -276,7 +278,7 @@
> return bump_resource(mc, relative, mc->vsn_url->data, pool);
> }
>
> -static int validate_element(void *userdata, svn_ra_dav__xml_elmid parent,
> +static int validate_element(svn_ra_dav__xml_elmid parent,
> svn_ra_dav__xml_elmid child)
> {
> if ((child == ELEM_collection || child == ELEM_baseline)
> @@ -371,11 +373,23 @@
> /* NOTREACHED */
> }
>
> -static int start_element(void *userdata, const svn_ra_dav__xml_elm_t *elm,
> - const char **atts)
> +static svn_error_t *
> +start_element(int *elem, void *baton, int parent,
> + const char *nspace, const char *name, const char **atts)
> {
> - merge_ctx_t *mc = userdata;
> + const svn_ra_dav__xml_elm_t *elm
> + = svn_ra_dav__lookup_xml_elem(merge_elements, nspace, name);
> + int acc = elm ? validate_element(parent, elm->id) : SVN_RA_DAV__XML_DECLINE;
> + merge_ctx_t *mc = baton;
> +
> + if (acc != SVN_RA_DAV__XML_VALID)
> + {
> + *elem = acc;
> + return (acc == SVN_RA_DAV__XML_DECLINE) ?
> + SVN_NO_ERROR : svn_error_create(SVN_ERR_XML_MALFORMED, NULL, NULL);
> + }
>
> + *elem = elm->id;
> switch (elm->id)
> {
> case ELEM_response:
> @@ -429,30 +443,49 @@
> break;
> }
>
> - return SVN_RA_DAV__XML_VALID;
> + switch (elm->id)
> + {
> + case ELEM_href:
> + case ELEM_status:
> + case ELEM_version_name:
> + case ELEM_post_commit_err:
> + case ELEM_creationdate:
> + case ELEM_creator_displayname:
> + mc->want_cdata = mc->cdata;
> + svn_stringbuf_setempty(mc->cdata);
> + break;
> +
> + default:
> + mc->want_cdata = NULL;
> + break;
> + }
> +
> +
> + return SVN_NO_ERROR;
> }
>
> -static int end_element(void *userdata, const svn_ra_dav__xml_elm_t *elm,
> - const char *cdata)
> +static svn_error_t *
> +end_element(void *baton, int state,
> + const char *nspace, const char *name)
> {
> - merge_ctx_t *mc = userdata;
> + merge_ctx_t *mc = baton;
>
> - switch (elm->id)
> + switch (state)
> {
> case ELEM_href:
> switch (mc->href_parent)
> {
> case ELEM_ignored_set:
> - add_ignored(mc, cdata);
> + add_ignored(mc, mc->cdata->data);
> break;
>
> case ELEM_response:
> /* we're now working on this href... */
> - svn_ra_dav__copy_href(mc->href, cdata);
> + svn_ra_dav__copy_href(mc->href, mc->cdata->data);
> break;
>
> case ELEM_checked_in:
> - svn_ra_dav__copy_href(mc->vsn_url, cdata);
> + svn_ra_dav__copy_href(mc->vsn_url, mc->cdata->data);
> break;
> }
> break;
> @@ -466,7 +499,7 @@
> {
> ne_status hs;
>
> - if (ne_parse_statusline(cdata, &hs) != 0)
> + if (ne_parse_statusline(mc->cdata->data, &hs) != 0)
> mc->response_has_error = TRUE;
> else
> {
> @@ -478,12 +511,12 @@
> }
> free(hs.reason_phrase);
> }
> - if (mc->response_has_error && mc->err == NULL)
> + if (mc->response_has_error)
> {
> /* ### fix this error value */
> - mc->err = svn_error_create(APR_EGENERAL, NULL,
> - _("The MERGE property response had an "
> - "error status"));
> + return svn_error_create(APR_EGENERAL, NULL,
> + _("The MERGE property response had an "
> + "error status"));
> }
> }
> break;
> @@ -499,20 +532,9 @@
>
> case ELEM_response:
> {
> - svn_error_t *err;
> -
> /* the end of a DAV:response means that we've seen all the information
> related to this resource. process it. */
> - err = handle_resource(mc, mc->scratchpool);
> - if (err != NULL)
> - {
> - /* ### how best to handle this error? for now, just remember the
> - ### first one found */
> - if (mc->err == NULL)
> - mc->err = err;
> - else
> - svn_error_clear(err);
> - }
> + SVN_ERR(handle_resource(mc, mc->scratchpool));
> svn_pool_clear(mc->scratchpool);
> }
> break;
> @@ -525,19 +547,19 @@
> break;
>
> case ELEM_version_name:
> - svn_stringbuf_set(mc->vsn_name, cdata);
> + svn_stringbuf_set(mc->vsn_name, mc->cdata->data);
> break;
>
> case ELEM_post_commit_err:
> - svn_stringbuf_set(mc->post_commit_err, cdata);
> + svn_stringbuf_set(mc->post_commit_err, mc->cdata->data);
> break;
>
> case ELEM_creationdate:
> - svn_stringbuf_set(mc->committed_date, cdata);
> + svn_stringbuf_set(mc->committed_date, mc->cdata->data);
> break;
>
> case ELEM_creator_displayname:
> - svn_stringbuf_set(mc->last_author, cdata);
> + svn_stringbuf_set(mc->last_author, mc->cdata->data);
> break;
>
> default:
> @@ -546,7 +568,7 @@
> break;
> }
>
> - return SVN_RA_DAV__XML_VALID;
> + return SVN_NO_ERROR;
> }
>
>
> @@ -678,8 +700,8 @@
> const char *body;
> apr_hash_t *extra_headers = NULL;
> svn_stringbuf_t *lockbuf = svn_stringbuf_create("", pool);
> - svn_error_t *err;
>
> + mc.cdata = svn_stringbuf_create("", pool);
> mc.pool = pool;
> mc.scratchpool = svn_pool_create(pool);
> mc.base_href = repos_url;
> @@ -734,20 +756,12 @@
> "</D:merge>",
> activity_url, lockbuf->data);
>
> - err = svn_ra_dav__parsed_request_compat(ras->sess, "MERGE", repos_url,
> - body, 0, NULL, merge_elements,
> - validate_element, start_element,
> - end_element, &mc, extra_headers,
> - NULL, FALSE, pool);
> -
> - /* is there an error stashed away in our context? */
> - if (mc.err)
> - {
> - svn_error_clear(err);
> - return mc.err;
> - }
> - else if (err)
> - return err;
> + SVN_ERR(svn_ra_dav__parsed_request(ras->sess, "MERGE", repos_url,
> + body, 0, NULL,
> + start_element,
> + svn_ra_dav__xml_collect_cdata,
> + end_element, &mc, extra_headers,
> + NULL, FALSE, pool));
>
> /* return some commit properties to the caller. */
> if (new_rev)
>
> Modified: trunk/subversion/libsvn_ra_dav/options.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_dav/options.c?pathrev=22514&r1=22513&r2=22514
> ==============================================================================
> --- trunk/subversion/libsvn_ra_dav/options.c (original)
> +++ trunk/subversion/libsvn_ra_dav/options.c Wed Nov 29 23:20:24 2006
> @@ -2,7 +2,7 @@
> * options.c : routines for performing OPTIONS server requests
> *
> * ====================================================================
> - * Copyright (c) 2000-2004 CollabNet. All rights reserved.
> + * Copyright (c) 2000-2006 CollabNet. All rights reserved.
> *
> * This software is licensed as described in the file COPYING, which
> * you should have received as part of this distribution. The terms
> @@ -40,15 +40,19 @@
> };
>
> typedef struct {
> - const svn_string_t *activity_coll;
> + /*WARNING: WANT_CDATA should stay the first element in the baton:
> + svn_ra_dav__xml_collect_cdata() assumes the baton starts with a stringbuf.
> + */
> + svn_stringbuf_t *want_cdata;
> + svn_stringbuf_t *cdata;
> apr_pool_t *pool;
> -
> + svn_string_t *activity_coll;
> } options_ctx_t;
>
>
>
> -static int validate_element(void *userdata, svn_ra_dav__xml_elmid parent,
> - svn_ra_dav__xml_elmid child)
> +static int
> +validate_element(svn_ra_dav__xml_elmid parent, svn_ra_dav__xml_elmid child)
> {
> switch (parent)
> {
> @@ -77,31 +81,49 @@
> /* NOTREACHED */
> }
>
> -static int start_element(void *userdata, const svn_ra_dav__xml_elm_t *elm,
> - const char **atts)
> +static svn_error_t *
> +start_element(int *elem, void *baton, int parent,
> + const char *nspace, const char *name, const char **atts)
> {
> - /* nothing to do here */
> - return SVN_RA_DAV__XML_VALID;
> + options_ctx_t *oc = baton;
> + const svn_ra_dav__xml_elm_t *elm
> + = svn_ra_dav__lookup_xml_elem(options_elements, nspace, name);
> + int acc = elm ? validate_element(parent, elm->id) : SVN_RA_DAV__XML_DECLINE;
> +
> + if (acc != SVN_RA_DAV__XML_VALID)
> + {
> + *elem = acc;
> + return (acc == SVN_RA_DAV__XML_DECLINE) ?
> + SVN_NO_ERROR : svn_error_create(SVN_ERR_XML_MALFORMED, NULL, NULL);
> + }
> + else
> + *elem = elm->id;
> +
> + if (elm->id == ELEM_href)
> + oc->want_cdata = oc->cdata;
> + else
> + oc->want_cdata = NULL;
> +
> + return SVN_NO_ERROR;
> }
>
> -static int end_element(void *userdata, const svn_ra_dav__xml_elm_t *elm,
> - const char *cdata)
> +static svn_error_t *
> +end_element(void *baton, int state,
> + const char *nspace, const char *name)
> {
> - options_ctx_t *oc = userdata;
> + options_ctx_t *oc = baton;
>
> - if (elm->id == ELEM_href)
> - {
> - oc->activity_coll = svn_string_create(cdata, oc->pool);
> - }
> + if (state == ELEM_href)
> + oc->activity_coll = svn_string_create_from_buf(oc->cdata, oc->pool);
>
> - return SVN_RA_DAV__XML_VALID;
> + return SVN_NO_ERROR;
> }
>
> -svn_error_t * svn_ra_dav__get_activity_collection(
> - const svn_string_t **activity_coll,
> - svn_ra_dav__session_t *ras,
> - const char *url,
> - apr_pool_t *pool)
> +svn_error_t *
> +svn_ra_dav__get_activity_collection(const svn_string_t **activity_coll,
> + svn_ra_dav__session_t *ras,
> + const char *url,
> + apr_pool_t *pool)
> {
> options_ctx_t oc = { 0 };
>
> @@ -111,17 +133,18 @@
> #endif
>
> oc.pool = pool;
> + oc.cdata = svn_stringbuf_create("", pool);
>
> - SVN_ERR(svn_ra_dav__parsed_request_compat(ras->sess, "OPTIONS", url,
> - "<?xml version=\"1.0\" "
> - "encoding=\"utf-8\"?>"
> - "<D:options xmlns:D=\"DAV:\">"
> - "<D:activity-collection-set/>"
> - "</D:options>", 0, NULL,
> - options_elements,
> - validate_element, start_element,
> - end_element, &oc,
> - NULL, NULL, FALSE, pool));
> + SVN_ERR(svn_ra_dav__parsed_request(ras->sess, "OPTIONS", url,
> + "<?xml version=\"1.0\" "
> + "encoding=\"utf-8\"?>"
> + "<D:options xmlns:D=\"DAV:\">"
> + "<D:activity-collection-set/>"
> + "</D:options>", 0, NULL,
> + start_element,
> + svn_ra_dav__xml_collect_cdata,
> + end_element, &oc,
> + NULL, NULL, FALSE, pool));
>
> if (oc.activity_coll == NULL)
> {
>
> Modified: trunk/subversion/libsvn_ra_dav/ra_dav.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_dav/ra_dav.h?pathrev=22514&r1=22513&r2=22514
> ==============================================================================
> --- trunk/subversion/libsvn_ra_dav/ra_dav.h (original)
> +++ trunk/subversion/libsvn_ra_dav/ra_dav.h Wed Nov 29 23:20:24 2006
> @@ -117,8 +117,6 @@
>
>
> 
> -
> -
> /* Context for neon request hooks; shared by the neon callbacks in
> session.c. */
> struct lock_request_baton
> @@ -614,6 +612,17 @@
> const char *name);
>
>
> +
> +/* Collect CDATA into a stringbuf.
> + *
> + * BATON points to a struct of which the first element is
> + * assumed to be an svn_stringbuf_t *.
> + */
> +svn_error_t *
> +svn_ra_dav__xml_collect_cdata(void *baton, int state,
> + const char *cdata, size_t len);
> +
> +
> /* Our equivalent of ne_xml_startelm_cb, the difference being that it
> * returns errors in a svn_error_t, and returns the element type via
> * ELEM. To ignore the element *ELEM should be set to NE_XML_DECLINE
>
> Modified: trunk/subversion/libsvn_ra_dav/util.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_dav/util.c?pathrev=22514&r1=22513&r2=22514
> ==============================================================================
> --- trunk/subversion/libsvn_ra_dav/util.c (original)
> +++ trunk/subversion/libsvn_ra_dav/util.c Wed Nov 29 23:20:24 2006
> @@ -337,6 +337,17 @@
> }
>
>
> +svn_error_t *
> +svn_ra_dav__xml_collect_cdata(void *baton, int state,
> + const char *cdata, size_t len)
> +{
> + svn_stringbuf_t **b = baton;
> +
> + if (*b)
> + svn_stringbuf_appendbytes(*b, cdata, len);
> +
> + return SVN_NO_ERROR;
> +}
> 
>
> void svn_ra_dav__copy_href(svn_stringbuf_t *dst, const char *src)
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org