[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [WIP] Gracefully handle svn_wc_prop_set4() errors in svn patch

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Tue, 28 Sep 2010 15:45:33 +0200

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.)

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.

>
> 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);
> + }
> +
> for (i = 0; i < prop_target->content_info->hunks->nelts; i++)
> {
> const hunk_info_t *hi;
> @@ -2189,6 +2211,7 @@ install_patched_prop_targets(patch_target_t *targe
> svn_stringbuf_t *prop_content;
> const char *eol_str;
> svn_boolean_t eof;
> + svn_error_t *err;
>
> svn_pool_clear(iterpool);
>
> @@ -2260,14 +2283,23 @@ install_patched_prop_targets(patch_target_t *targe
> * ### stsp: I'd say reject the property hunk.
> * ### We should verify all modified prop hunk texts using
> * ### svn_wc_canonicalize_svn_prop() before starting the
> - * ### patching process, to reject them as early as possible. */
> - SVN_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));
> + * ### patching process, to reject them as early as possible.
> + *
> + * ### dannas: Unfortunately we need the prop_content to run
> + * ### svn_wc_canonicalize_svn_prop() and we don't have that
> + * ### until we've applied our text changes. */
> + 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".)

> + }
> }
>
> svn_pool_destroy(iterpool);
Received on 2010-09-28 15:46:56 CEST

This is an archived mail posted to the Subversion Dev mailing list.