On Tue, Sep 28, 2010 at 03:45:33PM +0200, Daniel Shahaf wrote:
> Daniel Näslund wrote on Mon, Sep 27, 2010 at 22:09:26 +0200:
> > * What is wrong with the way I handle the error? I hit err_abort() when
> > the program terminates. (I'm expecting the answer to hurt a bit - this
> > is surely something that I should have understood by now). I thought
> > that since the error is allocated on the heap I could just store the
> > pointer to it and free it later, e.g. call svn_error_clear().
>
> err_abort() is called when an error object hadn't been svn_error_clear()'d.
> (The error creation installed err_abort() as a pool cleanup callback,
> and clearing the error unregisters the callback.)
Yes, that was how I understood it.
> So, yeah, you can do whatever you want with the error (they get
> allocated in a global pool) as long as you svn_error_clear() them
> eventually.
Ok.
> > Index: subversion/svn/notify.c
> > ===================================================================
> > --- subversion/svn/notify.c (revision 1001829)
> > +++ subversion/svn/notify.c (arbetskopia)
> > @@ -464,6 +464,16 @@ notify(void *baton, const svn_wc_notify_t *n, apr_
> > goto print_error;
> > break;
> >
> > + case svn_wc_notify_failed_prop_patch:
> > + nb->received_some_change = TRUE;
> > + err = svn_cmdline_printf(pool,
> > + _("property '%s' rejected from '%s'.\n"),
> > + n->prop_name, path_local);
> > + svn_handle_warning2(stderr, n->err, "svn: ");
> > + if (err)
> > + goto print_error;
> > + break;
> > +
>
> That's fine, print_error: clears the error.
> > Index: subversion/libsvn_client/patch.c
> > ===================================================================
> > --- subversion/libsvn_client/patch.c (revision 1001829)
> > +++ subversion/libsvn_client/patch.c (arbetskopia)
> > @@ -130,6 +130,12 @@ typedef struct prop_patch_target_t {
> >
> > /* ### Here we'll add flags telling if the prop was added, deleted,
> > * ### had_rejects, had_local_mods prior to patching and so on. */
> > +
> > + /* TRUE if the property could not be set on the path. */
> > + svn_boolean_t was_rejected;
> > +
> > + /* Set if was_rejected is TRUE. */
> > + svn_error_t *err;
> > } prop_patch_target_t;
> >
> > typedef struct patch_target_t {
> > @@ -1573,6 +1579,22 @@ send_patch_notification(const patch_target_t *targ
> >
> > prop_target = svn__apr_hash_index_val(hash_index);
> >
> > + if (prop_target->was_rejected)
> > + {
> > + svn_wc_notify_t *notify;
> > + svn_wc_notify_action_t action =
> > svn_wc_notify_failed_prop_patch;
> > +
> > + notify = svn_wc_create_notify(target->local_abspath
> > + ? target->local_abspath
> > + : target->local_relpath,
> > + action, pool);
> > + notify->prop_name = prop_target->name;
> > + notify->err = prop_target->err;
> > +
> > + (*ctx->notify_func2)(ctx->notify_baton2, notify, pool);
> > + svn_error_clear(prop_target->err);
Here I'm clearing prop_target->err. Since prop_target->was_rejected is
only set if and error exists (e.g. ! prop_target->err) I was expecting
that err would always be cleared.
[...]
> > @@ -2260,14 +2283,23 @@ install_patched_prop_targets(patch_target_t *targe
> > + err = svn_wc_prop_set4(ctx->wc_ctx, target->local_abspath,
> > + prop_target->name,
> > + svn_string_create_from_buf(prop_content,
> > + iterpool),
> > + TRUE /* skip_checks */,
> > + NULL, NULL,
> > + iterpool);
> > + if (err)
> > + {
> > + prop_target->was_rejected = TRUE;
> > + prop_target->err = err;
>
> Does prop_target->err always get cleared?
>
> (The answer is probably "No".)
As I said above, my intention was to clear it in
send_patch_notification().
I'll check again and see if I can spot why the err isn't always cleared.
Thanks for your feedback,
Daniel (dannas)
