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

Re: svn commit: r12937 - branches/locking/subversion/libsvn_ra_dav

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2005-02-08 18:43:53 CET

sussman@tigris.org writes:

> Author: sussman
> Date: Mon Feb 7 17:20:22 2005
> New Revision: 12937
>
> Modified:
> branches/locking/subversion/libsvn_ra_dav/fetch.c
> branches/locking/subversion/libsvn_ra_dav/ra_dav.h
> branches/locking/subversion/libsvn_ra_dav/session.c
> Log:
> Locking branch: implement client part of new REPORT for svn_repos_get_locks().
>
> * libsvn_ra_dav/ra_dav.h

Paths usually go further back:

    subversion/libsvn_ra_dav/ra_dav.h

> (enum): add a bunch of new ELEM_* element to the long enum for new report.
> (svn_ra_dav__get_locks): declare.
>
> * libsvn_ra_dav/session.c
> (svn_ra_dav__get_locks): remove the old stub.
>
> * libsvn_ra_dav/fetch.c
> (getlocks_report_elements): new array of elements for new report.
> (get_locks_baton_t): new baton structure.
> (getlocks_start_element,
> getlocks_cdata_handler,
> getlocks_end_element): new xml parsing callbacks.
> (svn_ra_dav__get_locks): new function.

> +/* This implements the `ne_xml_endelm_cb' prototype. */
> +static int getlocks_end_element(void *userdata, int state,
> + const char *ns, const char *ln)
> +{
> + get_locks_baton_t *baton = userdata;
> + const svn_ra_dav__xml_elm_t *elm;
> + svn_error_t *err;
> +
> + elm = svn_ra_dav__lookup_xml_elem(getlocks_report_elements, ns, ln);
> +
> + /* Just skip unknown elements. */
> + if (elm == NULL)
> + return NE_XML_DECLINE;
> +
> + switch (elm->id)
> + {
> + case ELEM_lock:
> + /* finish the whole svn_lock_t. */
> + apr_hash_set(baton->lock_hash, baton->current_lock->path,
> + APR_HASH_KEY_STRING, baton->current_lock);
> + break;
> +
> + case ELEM_lock_path:
> + /* neon has already xml-unescaped the cdata for us. */
> + baton->current_lock->path = apr_pstrdup(baton->pool,
> + baton->cdata_accum->data);
> + /* clean up the accumulator. */
> + svn_stringbuf_setempty(baton->cdata_accum);
> + svn_pool_clear(baton->scratchpool);

The stringbuf has been allocated in the scratch pool, nasty things are
about to happen--see below.

> + break;
> +
> + case ELEM_lock_token:
> + /* neon has already xml-unescaped the cdata for us. */
> + baton->current_lock->token = apr_pstrdup(baton->pool,
> + baton->cdata_accum->data);
> + /* clean up the accumulator. */
> + svn_stringbuf_setempty(baton->cdata_accum);
> + svn_pool_clear(baton->scratchpool);

ditto

> + break;
> +
> + case ELEM_lock_creationdate:
> + err = svn_time_from_cstring(&(baton->current_lock->creation_date),
> + baton->cdata_accum->data,
> + baton->scratchpool);
> + /* clean up the accumulator. */
> + svn_stringbuf_setempty(baton->cdata_accum);
> + svn_pool_clear(baton->scratchpool);

ditto, seems to be all the way through the function.

> + break;
> +
> + case ELEM_lock_expirationdate:
> + err = svn_time_from_cstring(&(baton->current_lock->expiration_date),
> + baton->cdata_accum->data,
> + baton->scratchpool);
> + /* clean up the accumulator. */
> + svn_stringbuf_setempty(baton->cdata_accum);
> + svn_pool_clear(baton->scratchpool);

ditto

> + break;
> +
> + case ELEM_lock_owner:
> + case ELEM_lock_comment:
> + {
> + const char *final_val;
> +
> + if (baton->encoding)
> + {
> + /* ### possibly recognize other encodings someday */
> + if (strcmp(baton->encoding, "base64") == 0)
> + {
> + svn_string_t *encoded_val;
> + const svn_string_t *decoded_val;
> +
> + encoded_val = svn_string_create_from_buf(baton->cdata_accum,
> + baton->scratchpool);
> + decoded_val = svn_base64_decode_string(encoded_val,
> + baton->scratchpool);
> + final_val = decoded_val->data;
> + }
> + else
> + /* unrecognized encoding! */
> + return NE_XML_ABORT;
> +
> + baton->encoding = NULL;
> + }
> + else
> + {
> + /* neon has already xml-unescaped the cdata for us. */
> + final_val = baton->cdata_accum->data;
> + }
> +
> + if (elm->id == ELEM_lock_owner)
> + baton->current_lock->owner = apr_pstrdup(baton->pool, final_val);
> + if (elm->id == ELEM_lock_comment)
> + baton->current_lock->comment = apr_pstrdup(baton->pool, final_val);
> +
> + /* clean up the accumulator. */
> + svn_stringbuf_setempty(baton->cdata_accum);
> + svn_pool_clear(baton->scratchpool);

ditto

> + break;
> + }
> +
> +
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +
> +
> +svn_error_t *
> +svn_ra_dav__get_locks(svn_ra_session_t *session,
> + apr_hash_t **locks,
> + const char *path,
> + apr_pool_t *pool)
> +{
> + svn_ra_dav__session_t *ras = session->priv;
> + const char *body, *url;
> + svn_error_t *err;
> + int status_code = 0;
> + get_locks_baton_t *baton;
> +
> + baton = apr_pcalloc(pool, sizeof(*baton));
> + baton->lock_hash = apr_hash_make (pool);
> + baton->pool = pool;
> + baton->scratchpool = svn_pool_create(pool);
> + baton->cdata_accum = svn_stringbuf_create("", baton->scratchpool);

This is the problem: the *cdata_accum stringbuf is allocated from the
scratchpool and so when scratchpool is cleared *cdata_accum is no
longer valid. Perhaps you should use an svn_stringbuf_t rather than
an svn_stringbuf_t* in get_locks_baton_t?

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Feb 8 18:45:00 2005

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