You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Rall <dl...@collab.net> on 2006/11/30 17:55:53 UTC

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