sussman@tigris.org writes:
> +/* Shared helper func: given a public URL which may not exist in HEAD,
> + search up parent directories until we can retrieve a *RSRC
> + containing a standard set of "starting" props: {VCC, resourcetype,
> + baseline-relative-path}. Also return *MISSING_PATH, which is the
> + trailing portion of the URL that did not exist. */
> +static svn_error_t * search_for_starting_props(svn_ra_dav_resource_t **rsrc,
> + const char **missing_path,
> + ne_session *sess,
> + const char *url,
> + apr_pool_t *pool)
Is conventional not to bother to document SESS in these sorts of
functions? (That's not meant to sound sarcastic, I just noticed that
a parameter isn't doc'd and thought I'd ask if it was deliberate.)
You don't say whether *MISSING_PATH's storage is shared with URL, or
whether it is copied into POOL. I could see it going either way, just
looking at the interface of the function (unless I really think about
it, in which case I realize the 'const'-ness of URL makes that
difficult, though not impossible :-) ).
Presumably that *RSRC gets allocated in POOL, not in "SESS's pool" or
anything odd like that? Might want to state so explicitly.
> +{
> + svn_error_t *err;
> + apr_size_t len;
> + svn_stringbuf_t *path_s;
> + ne_uri parsed_url;
> + const char *lopped_path = "";
> +
> + /* Split the url into it's component pieces (schema, host, path,
> + etc). We want the path part. */
> + ne_uri_parse (url, &parsed_url);
> +
> + path_s = svn_stringbuf_create (parsed_url.path, pool);
> +
> + /* Try to get the starting_props from the public url. If the
> + resource no longer exists in HEAD, we'll get a failure. That's
> + fine: just keep removing components and trying to get the
> + starting_props from parent directories. */
> +
> + while (! svn_path_is_empty (path_s->data))
> + {
> + err = svn_ra_dav__get_starting_props(rsrc, sess, path_s->data,
> + NULL, pool);
> + if (! err)
> + break; /* found an existing parent! */
> +
> + if (err->apr_err != SVN_ERR_RA_DAV_REQUEST_FAILED)
> + return err; /* found a _real_ error */
> +
> + /* else... lop off the basename and try again. */
> + lopped_path = svn_path_join(svn_path_basename (path_s->data, pool),
> + lopped_path,
> + pool);
> + len = path_s->len;
> + svn_path_remove_component(path_s);
> + if (path_s->len == len)
> + /* whoa, infinite loop, get out. */
> + return svn_error_quick_wrap(err,
> + "The path was not part of a repository");
> +
> + svn_error_clear (err);
> + }
Hmmm. I was about to say something about how much that could be
optimized, but then you and Mike pointed that each iteration of the
loop *contacts a freakin' DAV server*. Given that, saving a few
cycles or bytes here probably doesn't matter so much :-).
> + *missing_path = lopped_path;
> + return SVN_NO_ERROR;
> +}
Doc string might want to promise that *MISSING_PATH isn't touched if
any error is returned (or, if that promise isn't important, then you
don't need the lopped_path variable at all I suppose, you can just use
*missing_path itself).
> +svn_error_t *svn_ra_dav__get_vcc(const char **vcc,
> + ne_session *sess,
> + const char *url,
> + apr_pool_t *pool)
> +{
> + svn_ra_dav_resource_t *rsrc;
> + const char *lopped_path;
> + const svn_string_t *vcc_s;
> +
> + /* ### Someday, possibly look for memory-cached VCC in the RA session. */
> +
> + /* ### Someday, possibly look for disk-cached VCC via get_wcprop callback. */
> +
> + /* Finally, resort to a set of PROPFINDs up parent directories. */
> + SVN_ERR( search_for_starting_props(&rsrc, &lopped_path,
> + sess, url, pool) );
Isn't it kind of odd for the first call in a function to start with a
comment saying "Finally, ..." :-) ?
> +/* Fetch the repository's unique Version-Controlled-Configuration url.
> +
> + Given a Neon session SESS and a URL, set *VCC to the url of the
> + repository's version-controlled-configuration resource.
> + */
> +svn_error_t *svn_ra_dav__get_vcc(const char **vcc,
> + ne_session *sess,
> + const char *url,
> + apr_pool_t *pool);
>
> /* Issue a PROPPATCH request on URL, transmitting PROP_CHANGES (a hash
> of const svn_string_t * values keyed on Subversion user-visible
Same doc comments about POOL here, but I'm beginning to wonder if
there isn't a convention at work that makes them unnecessary (?).
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Apr 29 23:06:33 2003