You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Garrett Rooney <ro...@electricjellyfish.net> on 2006/04/08 05:41:42 UTC
Re: svn commit: r19251 - trunk/subversion/libsvn_ra_serf
On 4/7/06, jerenkrantz@tigris.org <je...@tigris.org> wrote:
> Author: jerenkrantz
> Date: Fri Apr 7 22:31:35 2006
> New Revision: 19251
>
> Added:
> trunk/subversion/libsvn_ra_serf/replay.c
> Modified:
> trunk/subversion/libsvn_ra_serf/commit.c
> trunk/subversion/libsvn_ra_serf/property.c
> trunk/subversion/libsvn_ra_serf/ra_serf.h
> trunk/subversion/libsvn_ra_serf/serf.c
>
> Log:
> ra_serf: Implement replay functionality; fix some session reuse bugs triggered
> by replay. (Passes all svnsync tests.)
Excellent! A few comments though...
> + else if (state == REPORT &&
> + strcmp(name.name, "open-root") == 0)
> + {
> + const char *rev;
> + replay_info_t *info;
> +
> + rev = svn_ra_serf__find_attr(attrs, "rev");
> +
> + info = push_state(parser, ctx, OPEN_DIR);
> +
> + SVN_ERR(ctx->editor->open_root(ctx->editor_baton,
> + SVN_STR_TO_REV(rev), parser->state->pool,
> + &info->baton));
It seems like there should be some error checking on the rev
attribute, what if it isn't there? Similar things could be said for
the rest of the editor calls in this file.
-garett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r19251 - trunk/subversion/libsvn_ra_serf
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
Garrett Rooney writes:
> On 4/7/06, Justin Erenkrantz <ju...@erenkrantz.com> wrote:
> > On Fri, Apr 07, 2006 at 10:41:42PM -0700, Garrett Rooney wrote:
> > > It seems like there should be some error checking on the rev
> > > attribute, what if it isn't there?
> >
> > It depends how much checking we think is appropriate. ra_dav and ra_serf
> > don't always generate errors on attribute omissions. *shrug*
> >
> > Look for "verify we got it. punt on error." in ra_dav/fetch.c. =)
>
> Sure, but when I implemented replay in ra_dav I did make sure to check
> for that sort of thing, so backsliding in ra_serf seems wrong ;-)
>
+1.
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r19251 - trunk/subversion/libsvn_ra_serf
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 4/7/06, Justin Erenkrantz <ju...@erenkrantz.com> wrote:
> On Fri, Apr 07, 2006 at 10:41:42PM -0700, Garrett Rooney wrote:
> > It seems like there should be some error checking on the rev
> > attribute, what if it isn't there?
>
> It depends how much checking we think is appropriate. ra_dav and ra_serf
> don't always generate errors on attribute omissions. *shrug*
>
> Look for "verify we got it. punt on error." in ra_dav/fetch.c. =)
Sure, but when I implemented replay in ra_dav I did make sure to check
for that sort of thing, so backsliding in ra_serf seems wrong ;-)
> > Similar things could be said for
> > the rest of the editor calls in this file.
>
> The editor calls are wrapped with SVN_ERR.
Sure, but are we sure all the places we pass potentially NULL
attribute strings actually deal with NULL arguments? Quite a few
places in svn explicitly don't, after all.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r19251 - trunk/subversion/libsvn_ra_serf
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Fri, Apr 07, 2006 at 10:41:42PM -0700, Garrett Rooney wrote:
> It seems like there should be some error checking on the rev
> attribute, what if it isn't there?
It depends how much checking we think is appropriate. ra_dav and ra_serf
don't always generate errors on attribute omissions. *shrug*
Look for "verify we got it. punt on error." in ra_dav/fetch.c. =)
> Similar things could be said for
> the rest of the editor calls in this file.
The editor calls are wrapped with SVN_ERR. -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org