Karl Fogel <kfogel@newton.ch.collab.net> writes:
> 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.)
Nah, just forgot to doc it. I'll fix that.
>
> 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 :-) ).
Will fix.
>
> Presumably that *RSRC gets allocated in POOL, not in "SESS's pool" or
> anything odd like that? Might want to state so explicitly.
There's a general problem here; this function calls a zillion other
ra_dav functions, *none* of which are documented. I guess I need to
trace all the way down the call stack and figure out who allocates
what. :-(
>
> > + *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).
Sure, makes sense.
> > +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, ..." :-) ?
Not if you read the first two comments. :-)
---------------------------------------------------------------------
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:30:22 2003