You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2010/08/05 06:12:32 UTC
Re: svn commit: r982415 - in
/subversion/trunk/subversion/libsvn_ra_serf: commit.c replay.c
serf.c update.c
rhuijben@apache.org wrote on Wed, Aug 04, 2010 at 22:20:30 -0000:
> Author: rhuijben
> Date: Wed Aug 4 22:20:30 2010
> New Revision: 982415
>
> URL: http://svn.apache.org/viewvc?rev=982415&view=rev
> Log:
> Following up on r982398, fix a few more mostly theoretical error leaks.
>
Thanks; review below.
> Modified: subversion/trunk/subversion/libsvn_ra_serf/commit.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/commit.c?rev=982415&r1=982414&r2=982415&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_serf/commit.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_serf/commit.c Wed Aug 4 22:20:30 2010
> @@ -748,6 +748,7 @@ create_proppatch_body(void *baton,
> {
> proppatch_context_t *ctx = baton;
> serf_bucket_t *body_bkt;
> + svn_error_t *err;
>
> body_bkt = serf_bucket_aggregate_create(alloc);
>
> @@ -764,9 +765,10 @@ create_proppatch_body(void *baton,
> svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:set", NULL);
> svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:prop", NULL);
>
> - svn_ra_serf__walk_all_props(ctx->changed_props, ctx->path,
> - SVN_INVALID_REVNUM,
> - proppatch_walker, body_bkt, pool);
> + err = svn_ra_serf__walk_all_props(ctx->changed_props, ctx->path,
> + SVN_INVALID_REVNUM,
> + proppatch_walker, body_bkt, pool);
> + svn_error_clear(err); /* ### */
>
What is the reason for silently ignoring errors here?
> svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:prop");
> svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:set");
> @@ -777,9 +779,10 @@ create_proppatch_body(void *baton,
> svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:remove", NULL);
> svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:prop", NULL);
>
> - svn_ra_serf__walk_all_props(ctx->removed_props, ctx->path,
> - SVN_INVALID_REVNUM,
> - proppatch_walker, body_bkt, pool);
> + err = svn_ra_serf__walk_all_props(ctx->removed_props, ctx->path,
> + SVN_INVALID_REVNUM,
> + proppatch_walker, body_bkt, pool);
> + svn_error_clear(err); /* ### */
>
Same question.
> svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:prop");
> svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:remove");
>
> Modified: subversion/trunk/subversion/libsvn_ra_serf/update.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/update.c?rev=982415&r1=982414&r2=982415&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_serf/update.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_serf/update.c Wed Aug 4 22:20:30 2010
> @@ -937,23 +941,25 @@ handle_fetch(serf_request_t *request,
> + if (!err && info->fetch_props)
> {
> - svn_ra_serf__walk_all_props(info->props,
> - info->url,
> - info->target_rev,
> - set_file_props,
> - info, info->editor_pool);
> + err = svn_ra_serf__walk_all_props(info->props,
> + info->url,
> + info->target_rev,
> + set_file_props,
> + info, info->editor_pool);
> }
>
> err = info->dir->update_editor->close_file(info->file_baton,
Leaks err.
Re: svn commit: r982415 - in
/subversion/trunk/subversion/libsvn_ra_serf: commit.c replay.c
serf.c update.c
Posted by 'Daniel Shahaf' <d....@daniel.shahaf.name>.
Bert Huijben wrote on Thu, Aug 05, 2010 at 08:50:15 +0200:
>
>
> > -----Original Message-----
> > From: Daniel Shahaf [mailto:d.s@daniel.shahaf.name]
> > Sent: donderdag 5 augustus 2010 8:13
> > To: dev@subversion.apache.org
> > Cc: commits@subversion.apache.org; Daniel Shahaf
> > Subject: Re: svn commit: r982415 - in
> > /subversion/trunk/subversion/libsvn_ra_serf: commit.c replay.c serf.c
> > update.c
> >
> > rhuijben@apache.org wrote on Wed, Aug 04, 2010 at 22:20:30 -0000:
> > > Author: rhuijben
> > > Date: Wed Aug 4 22:20:30 2010
> > > New Revision: 982415
> > >
> > > URL: http://svn.apache.org/viewvc?rev=982415&view=rev
> > > Log:
> > > Following up on r982398, fix a few more mostly theoretical error
> > leaks.
> > >
> >
> > Thanks; review below.
> >
> > > Modified: subversion/trunk/subversion/libsvn_ra_serf/commit.c
> > > URL:
> > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf
> > /commit.c?rev=982415&r1=982414&r2=982415&view=diff
> > >
> > =======================================================================
> > =======
> > > --- subversion/trunk/subversion/libsvn_ra_serf/commit.c (original)
> > > +++ subversion/trunk/subversion/libsvn_ra_serf/commit.c Wed Aug 4
> > 22:20:30 2010
> > > @@ -748,6 +748,7 @@ create_proppatch_body(void *baton,
> > > {
> > > proppatch_context_t *ctx = baton;
> > > serf_bucket_t *body_bkt;
> > > + svn_error_t *err;
> > >
> > > body_bkt = serf_bucket_aggregate_create(alloc);
> > >
> > > @@ -764,9 +765,10 @@ create_proppatch_body(void *baton,
> > > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:set",
> > NULL);
> > > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:prop",
> > NULL);
> > >
> > > - svn_ra_serf__walk_all_props(ctx->changed_props, ctx->path,
> > > - SVN_INVALID_REVNUM,
> > > - proppatch_walker, body_bkt, pool);
> > > + err = svn_ra_serf__walk_all_props(ctx->changed_props, ctx-
> > >path,
> > > + SVN_INVALID_REVNUM,
> > > + proppatch_walker, body_bkt,
> > pool);
> > > + svn_error_clear(err); /* ### */
> > >
> >
> > What is the reason for silently ignoring errors here?
>
> This is a serf callback which can't return errors. Clearing is better then
> leaking.
>
I disagree. If the walker did return an error, we can't be sure it's safe
to continue, so IMO we shouldn't.
We could clear the err and return NULL (or however this callback signals an
error to serf), or send the err to some global handler, or just
SVN_ERR_ASSERT_NO_RETURN(!err). But just ignoring&clearing it seems wrong.
(And it's going to be pretty nasty to debug this, I guess, if an error
should someday get thrown here.)
> >
> > > svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:prop");
> > > svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:set");
> > > @@ -777,9 +779,10 @@ create_proppatch_body(void *baton,
> > > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:remove",
> > NULL);
> > > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:prop",
> > NULL);
> > >
> > > - svn_ra_serf__walk_all_props(ctx->removed_props, ctx->path,
> > > - SVN_INVALID_REVNUM,
> > > - proppatch_walker, body_bkt, pool);
> > > + err = svn_ra_serf__walk_all_props(ctx->removed_props, ctx-
> > >path,
> > > + SVN_INVALID_REVNUM,
> > > + proppatch_walker, body_bkt,
> > pool);
> > > + svn_error_clear(err); /* ### */
> > >
> >
> > Same question.
>
> Same function.
Same disagreement.
Re: svn commit: r982415 - in
/subversion/trunk/subversion/libsvn_ra_serf: commit.c replay.c
serf.c update.c
Posted by 'Daniel Shahaf' <d....@daniel.shahaf.name>.
Bert Huijben wrote on Thu, Aug 05, 2010 at 08:50:15 +0200:
>
>
> > -----Original Message-----
> > From: Daniel Shahaf [mailto:d.s@daniel.shahaf.name]
> > Sent: donderdag 5 augustus 2010 8:13
> > To: dev@subversion.apache.org
> > Cc: commits@subversion.apache.org; Daniel Shahaf
> > Subject: Re: svn commit: r982415 - in
> > /subversion/trunk/subversion/libsvn_ra_serf: commit.c replay.c serf.c
> > update.c
> >
> > rhuijben@apache.org wrote on Wed, Aug 04, 2010 at 22:20:30 -0000:
> > > Author: rhuijben
> > > Date: Wed Aug 4 22:20:30 2010
> > > New Revision: 982415
> > >
> > > URL: http://svn.apache.org/viewvc?rev=982415&view=rev
> > > Log:
> > > Following up on r982398, fix a few more mostly theoretical error
> > leaks.
> > >
> >
> > Thanks; review below.
> >
> > > Modified: subversion/trunk/subversion/libsvn_ra_serf/commit.c
> > > URL:
> > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf
> > /commit.c?rev=982415&r1=982414&r2=982415&view=diff
> > >
> > =======================================================================
> > =======
> > > --- subversion/trunk/subversion/libsvn_ra_serf/commit.c (original)
> > > +++ subversion/trunk/subversion/libsvn_ra_serf/commit.c Wed Aug 4
> > 22:20:30 2010
> > > @@ -748,6 +748,7 @@ create_proppatch_body(void *baton,
> > > {
> > > proppatch_context_t *ctx = baton;
> > > serf_bucket_t *body_bkt;
> > > + svn_error_t *err;
> > >
> > > body_bkt = serf_bucket_aggregate_create(alloc);
> > >
> > > @@ -764,9 +765,10 @@ create_proppatch_body(void *baton,
> > > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:set",
> > NULL);
> > > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:prop",
> > NULL);
> > >
> > > - svn_ra_serf__walk_all_props(ctx->changed_props, ctx->path,
> > > - SVN_INVALID_REVNUM,
> > > - proppatch_walker, body_bkt, pool);
> > > + err = svn_ra_serf__walk_all_props(ctx->changed_props, ctx-
> > >path,
> > > + SVN_INVALID_REVNUM,
> > > + proppatch_walker, body_bkt,
> > pool);
> > > + svn_error_clear(err); /* ### */
> > >
> >
> > What is the reason for silently ignoring errors here?
>
> This is a serf callback which can't return errors. Clearing is better then
> leaking.
>
I disagree. If the walker did return an error, we can't be sure it's safe
to continue, so IMO we shouldn't.
We could clear the err and return NULL (or however this callback signals an
error to serf), or send the err to some global handler, or just
SVN_ERR_ASSERT_NO_RETURN(!err). But just ignoring&clearing it seems wrong.
(And it's going to be pretty nasty to debug this, I guess, if an error
should someday get thrown here.)
> >
> > > svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:prop");
> > > svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:set");
> > > @@ -777,9 +779,10 @@ create_proppatch_body(void *baton,
> > > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:remove",
> > NULL);
> > > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:prop",
> > NULL);
> > >
> > > - svn_ra_serf__walk_all_props(ctx->removed_props, ctx->path,
> > > - SVN_INVALID_REVNUM,
> > > - proppatch_walker, body_bkt, pool);
> > > + err = svn_ra_serf__walk_all_props(ctx->removed_props, ctx-
> > >path,
> > > + SVN_INVALID_REVNUM,
> > > + proppatch_walker, body_bkt,
> > pool);
> > > + svn_error_clear(err); /* ### */
> > >
> >
> > Same question.
>
> Same function.
Same disagreement.
Re: svn commit: r982415 - in
/subversion/trunk/subversion/libsvn_ra_serf: commit.c replay.c
serf.c update.c
Posted by 'Daniel Shahaf' <d....@daniel.shahaf.name>.
Bert Huijben wrote on Thu, Aug 05, 2010 at 08:50:15 +0200:
>
>
> > -----Original Message-----
> > From: Daniel Shahaf [mailto:d.s@daniel.shahaf.name]
> > Sent: donderdag 5 augustus 2010 8:13
> > To: dev@subversion.apache.org
> > Cc: commits@subversion.apache.org; Daniel Shahaf
> > Subject: Re: svn commit: r982415 - in
> > /subversion/trunk/subversion/libsvn_ra_serf: commit.c replay.c serf.c
> > update.c
> >
> > rhuijben@apache.org wrote on Wed, Aug 04, 2010 at 22:20:30 -0000:
> > > Author: rhuijben
> > > Date: Wed Aug 4 22:20:30 2010
> > > New Revision: 982415
> > >
> > > URL: http://svn.apache.org/viewvc?rev=982415&view=rev
> > > Log:
> > > Following up on r982398, fix a few more mostly theoretical error
> > leaks.
> > >
> >
> > Thanks; review below.
> >
> > > Modified: subversion/trunk/subversion/libsvn_ra_serf/commit.c
> > > URL:
> > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf
> > /commit.c?rev=982415&r1=982414&r2=982415&view=diff
> > >
> > =======================================================================
> > =======
> > > --- subversion/trunk/subversion/libsvn_ra_serf/commit.c (original)
> > > +++ subversion/trunk/subversion/libsvn_ra_serf/commit.c Wed Aug 4
> > 22:20:30 2010
> > > @@ -748,6 +748,7 @@ create_proppatch_body(void *baton,
> > > {
> > > proppatch_context_t *ctx = baton;
> > > serf_bucket_t *body_bkt;
> > > + svn_error_t *err;
> > >
> > > body_bkt = serf_bucket_aggregate_create(alloc);
> > >
> > > @@ -764,9 +765,10 @@ create_proppatch_body(void *baton,
> > > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:set",
> > NULL);
> > > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:prop",
> > NULL);
> > >
> > > - svn_ra_serf__walk_all_props(ctx->changed_props, ctx->path,
> > > - SVN_INVALID_REVNUM,
> > > - proppatch_walker, body_bkt, pool);
> > > + err = svn_ra_serf__walk_all_props(ctx->changed_props, ctx-
> > >path,
> > > + SVN_INVALID_REVNUM,
> > > + proppatch_walker, body_bkt,
> > pool);
> > > + svn_error_clear(err); /* ### */
> > >
> >
> > What is the reason for silently ignoring errors here?
>
> This is a serf callback which can't return errors. Clearing is better then
> leaking.
>
I disagree. If the walker did return an error, we can't be sure it's safe
to continue, so IMO we shouldn't.
We could clear the err and return NULL (or however this callback signals an
error to serf), or send the err to some global handler, or just
SVN_ERR_ASSERT_NO_RETURN(!err). But just ignoring&clearing it seems wrong.
(And it's going to be pretty nasty to debug this, I guess, if an error
should someday get thrown here.)
> >
> > > svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:prop");
> > > svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:set");
> > > @@ -777,9 +779,10 @@ create_proppatch_body(void *baton,
> > > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:remove",
> > NULL);
> > > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:prop",
> > NULL);
> > >
> > > - svn_ra_serf__walk_all_props(ctx->removed_props, ctx->path,
> > > - SVN_INVALID_REVNUM,
> > > - proppatch_walker, body_bkt, pool);
> > > + err = svn_ra_serf__walk_all_props(ctx->removed_props, ctx-
> > >path,
> > > + SVN_INVALID_REVNUM,
> > > + proppatch_walker, body_bkt,
> > pool);
> > > + svn_error_clear(err); /* ### */
> > >
> >
> > Same question.
>
> Same function.
Same disagreement.
RE: svn commit: r982415 - in /subversion/trunk/subversion/libsvn_ra_serf: commit.c replay.c serf.c update.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Daniel Shahaf [mailto:d.s@daniel.shahaf.name]
> Sent: donderdag 5 augustus 2010 8:13
> To: dev@subversion.apache.org
> Cc: commits@subversion.apache.org; Daniel Shahaf
> Subject: Re: svn commit: r982415 - in
> /subversion/trunk/subversion/libsvn_ra_serf: commit.c replay.c serf.c
> update.c
>
> rhuijben@apache.org wrote on Wed, Aug 04, 2010 at 22:20:30 -0000:
> > Author: rhuijben
> > Date: Wed Aug 4 22:20:30 2010
> > New Revision: 982415
> >
> > URL: http://svn.apache.org/viewvc?rev=982415&view=rev
> > Log:
> > Following up on r982398, fix a few more mostly theoretical error
> leaks.
> >
>
> Thanks; review below.
>
> > Modified: subversion/trunk/subversion/libsvn_ra_serf/commit.c
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf
> /commit.c?rev=982415&r1=982414&r2=982415&view=diff
> >
> =======================================================================
> =======
> > --- subversion/trunk/subversion/libsvn_ra_serf/commit.c (original)
> > +++ subversion/trunk/subversion/libsvn_ra_serf/commit.c Wed Aug 4
> 22:20:30 2010
> > @@ -748,6 +748,7 @@ create_proppatch_body(void *baton,
> > {
> > proppatch_context_t *ctx = baton;
> > serf_bucket_t *body_bkt;
> > + svn_error_t *err;
> >
> > body_bkt = serf_bucket_aggregate_create(alloc);
> >
> > @@ -764,9 +765,10 @@ create_proppatch_body(void *baton,
> > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:set",
> NULL);
> > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:prop",
> NULL);
> >
> > - svn_ra_serf__walk_all_props(ctx->changed_props, ctx->path,
> > - SVN_INVALID_REVNUM,
> > - proppatch_walker, body_bkt, pool);
> > + err = svn_ra_serf__walk_all_props(ctx->changed_props, ctx-
> >path,
> > + SVN_INVALID_REVNUM,
> > + proppatch_walker, body_bkt,
> pool);
> > + svn_error_clear(err); /* ### */
> >
>
> What is the reason for silently ignoring errors here?
This is a serf callback which can't return errors. Clearing is better then
leaking.
>
> > svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:prop");
> > svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:set");
> > @@ -777,9 +779,10 @@ create_proppatch_body(void *baton,
> > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:remove",
> NULL);
> > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:prop",
> NULL);
> >
> > - svn_ra_serf__walk_all_props(ctx->removed_props, ctx->path,
> > - SVN_INVALID_REVNUM,
> > - proppatch_walker, body_bkt, pool);
> > + err = svn_ra_serf__walk_all_props(ctx->removed_props, ctx-
> >path,
> > + SVN_INVALID_REVNUM,
> > + proppatch_walker, body_bkt,
> pool);
> > + svn_error_clear(err); /* ### */
> >
>
> Same question.
Same function.
>
> > svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:prop");
> > svn_ra_serf__add_close_tag_buckets(body_bkt, alloc,
> "D:remove");
> >
> > Modified: subversion/trunk/subversion/libsvn_ra_serf/update.c
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf
> /update.c?rev=982415&r1=982414&r2=982415&view=diff
> >
> =======================================================================
> =======
> > --- subversion/trunk/subversion/libsvn_ra_serf/update.c (original)
> > +++ subversion/trunk/subversion/libsvn_ra_serf/update.c Wed Aug 4
> 22:20:30 2010
> > @@ -937,23 +941,25 @@ handle_fetch(serf_request_t *request,
> > + if (!err && info->fetch_props)
> > {
> > - svn_ra_serf__walk_all_props(info->props,
> > - info->url,
> > - info->target_rev,
> > - set_file_props,
> > - info, info->editor_pool);
> > + err = svn_ra_serf__walk_all_props(info->props,
> > + info->url,
> > + info->target_rev,
> > + set_file_props,
> > + info, info-
> >editor_pool);
> > }
> >
> > err = info->dir->update_editor->close_file(info-
> >file_baton,
>
> Leaks err.
I'll fix this one.
Bert
RE: svn commit: r982415 - in /subversion/trunk/subversion/libsvn_ra_serf: commit.c replay.c serf.c update.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Daniel Shahaf [mailto:d.s@daniel.shahaf.name]
> Sent: donderdag 5 augustus 2010 8:13
> To: dev@subversion.apache.org
> Cc: commits@subversion.apache.org; Daniel Shahaf
> Subject: Re: svn commit: r982415 - in
> /subversion/trunk/subversion/libsvn_ra_serf: commit.c replay.c serf.c
> update.c
>
> rhuijben@apache.org wrote on Wed, Aug 04, 2010 at 22:20:30 -0000:
> > Author: rhuijben
> > Date: Wed Aug 4 22:20:30 2010
> > New Revision: 982415
> >
> > URL: http://svn.apache.org/viewvc?rev=982415&view=rev
> > Log:
> > Following up on r982398, fix a few more mostly theoretical error
> leaks.
> >
>
> Thanks; review below.
>
> > Modified: subversion/trunk/subversion/libsvn_ra_serf/commit.c
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf
> /commit.c?rev=982415&r1=982414&r2=982415&view=diff
> >
> =======================================================================
> =======
> > --- subversion/trunk/subversion/libsvn_ra_serf/commit.c (original)
> > +++ subversion/trunk/subversion/libsvn_ra_serf/commit.c Wed Aug 4
> 22:20:30 2010
> > @@ -748,6 +748,7 @@ create_proppatch_body(void *baton,
> > {
> > proppatch_context_t *ctx = baton;
> > serf_bucket_t *body_bkt;
> > + svn_error_t *err;
> >
> > body_bkt = serf_bucket_aggregate_create(alloc);
> >
> > @@ -764,9 +765,10 @@ create_proppatch_body(void *baton,
> > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:set",
> NULL);
> > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:prop",
> NULL);
> >
> > - svn_ra_serf__walk_all_props(ctx->changed_props, ctx->path,
> > - SVN_INVALID_REVNUM,
> > - proppatch_walker, body_bkt, pool);
> > + err = svn_ra_serf__walk_all_props(ctx->changed_props, ctx-
> >path,
> > + SVN_INVALID_REVNUM,
> > + proppatch_walker, body_bkt,
> pool);
> > + svn_error_clear(err); /* ### */
> >
>
> What is the reason for silently ignoring errors here?
This is a serf callback which can't return errors. Clearing is better then
leaking.
>
> > svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:prop");
> > svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:set");
> > @@ -777,9 +779,10 @@ create_proppatch_body(void *baton,
> > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:remove",
> NULL);
> > svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:prop",
> NULL);
> >
> > - svn_ra_serf__walk_all_props(ctx->removed_props, ctx->path,
> > - SVN_INVALID_REVNUM,
> > - proppatch_walker, body_bkt, pool);
> > + err = svn_ra_serf__walk_all_props(ctx->removed_props, ctx-
> >path,
> > + SVN_INVALID_REVNUM,
> > + proppatch_walker, body_bkt,
> pool);
> > + svn_error_clear(err); /* ### */
> >
>
> Same question.
Same function.
>
> > svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:prop");
> > svn_ra_serf__add_close_tag_buckets(body_bkt, alloc,
> "D:remove");
> >
> > Modified: subversion/trunk/subversion/libsvn_ra_serf/update.c
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf
> /update.c?rev=982415&r1=982414&r2=982415&view=diff
> >
> =======================================================================
> =======
> > --- subversion/trunk/subversion/libsvn_ra_serf/update.c (original)
> > +++ subversion/trunk/subversion/libsvn_ra_serf/update.c Wed Aug 4
> 22:20:30 2010
> > @@ -937,23 +941,25 @@ handle_fetch(serf_request_t *request,
> > + if (!err && info->fetch_props)
> > {
> > - svn_ra_serf__walk_all_props(info->props,
> > - info->url,
> > - info->target_rev,
> > - set_file_props,
> > - info, info->editor_pool);
> > + err = svn_ra_serf__walk_all_props(info->props,
> > + info->url,
> > + info->target_rev,
> > + set_file_props,
> > + info, info-
> >editor_pool);
> > }
> >
> > err = info->dir->update_editor->close_file(info-
> >file_baton,
>
> Leaks err.
I'll fix this one.
Bert