Re: svn commit: r30666 - trunk/subversion/libsvn_ra_neon

Karl Fogel wrote on Thu, 17 Apr 2008 at 15:40 -0400:
> kfogel_at_tigris.org writes:
> > Log:
> > In ra_neon, cache starting props when first seen, to reduce round trips.
> >
> > Patch by: Daniel Shahaf <d.s_at_daniel.shahaf.co.il>
> > (Tweaked by me.)
>
> Daniel, I promised I'd tell you what the tweaks were. Here they are:
>
>
> > --- trunk/subversion/libsvn_ra_neon/props.c (r30665)
> > +++ trunk/subversion/libsvn_ra_neon/props.c (r30666)
> > @@ -638,8 +638,32 @@ svn_error_t * svn_ra_neon__get_starting_
> > + if (! sess->uuid)
> > + {
> > + propval = apr_hash_get((*rsrc)->propset,
> > + SVN_RA_NEON__PROP_REPOSITORY_UUID,
> > + APR_HASH_KEY_STRING);
> > + if (propval)
> > + sess->uuid = apr_pstrdup(sess->pool, propval->data);
> > + }
> > +
> > + return SVN_NO_ERROR;
> > }
>
> As you can see, I rearranged the code a bit here, putting conditional
> checks earlier, using braces, etc.
>

OK, I missed that.

> > + if (! sess->vcc)
> > + {
> > + /* ### better error reporting... */
> > + return svn_error_create(APR_EGENERAL, NULL,
> > + _("The VCC property was not found on the "
> > + "resource"));
> > + }
>
> Added braces.
>

OK.

> > --- trunk/subversion/libsvn_ra_neon/ra_neon.h (r30665)
> > +++ trunk/subversion/libsvn_ra_neon/ra_neon.h (r30666)
> > @@ -98,6 +98,10 @@ typedef struct {
> > svn_auth_iterstate_t *p11pin_iterstate; /* state of PKCS#11 pin retries */
> >
> > svn_boolean_t compression; /* should we use http compression? */
> > +
> > + /* Both of these function as caches, and are NULL when uninitialized
> > + or cleared: */
> > + const char *vcc; /* version-controlled-configuration */
> > const char *uuid; /* repository UUID */
> >
> > svn_ra_progress_notify_func_t progress_func;
>
> Added the comment about how they're caches.
>

OK. The comments I put were not very helpful, indeed.

> > @@ -437,7 +444,10 @@ svn_error_t * svn_ra_neon__get_starting_
> >
> > Also return *MISSING_PATH (allocated in POOL), which is the
> > trailing portion of the URL that did not exist. If an error
> > - occurs, *MISSING_PATH isn't changed. */
> > + occurs, *MISSING_PATH isn't changed.
> > +
> > + Cache the version-controlled-configuration in SESS->vcc, and the
> > + repository uuid in SESS->uuid. */
> > svn_error_t *
> > svn_ra_neon__search_for_starting_props(svn_ra_neon__resource_t **rsrc,
> > const char **missing_path,
>
> Added documentation about the caching behavior.
>

OK.

> > --- trunk/subversion/libsvn_ra_neon/session.c (r30665)
> > +++ trunk/subversion/libsvn_ra_neon/session.c (r30666)
> > + if (! ras->uuid)
> > + {
> > + /* ### better error reporting... */
> > + return svn_error_create(APR_EGENERAL, NULL,
> > + _("The UUID property was not found on the "
> > + "resource or any of its parents"));
> > + }
> > }
>
> Added braces.
>
> -Karl
>

Thanks for the review Karl!

-- 
Daniel
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org