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

Re: permissions (and other) problems

From: Garrett Rooney <rooneg_at_electricjellyfish.net>
Date: 2002-09-13 15:16:18 CEST

Philip Martin wrote:

> Almost everywhere that ignores an error should use svn_error_clear_all,
> some places don't but that's usually a bug. We could change
>
> if (err)
> svn_error_clear_all (err);
>
> into
>
> if (err)
> SVN_ERR (svn_error_clear_all (err));
>
> and have svn_error_clear_all return an input ERR_CANCELED instead of
> clearing it.
>

yeah, that makes sense to me.

here's a first cut of this that i put together last night.

it uses functions inside SVN_ERR and SVN_ERR_W to figure out if we've
been canceled, rather than global variables, since for some reason no
matter what combination of declarations i tried, i wasn't able to get
everything to link without unresolved symbols.

there are actually two states we need to worry about. the first is
'have we been canceled', which tells SVN_ERR and SVN_ERR_W to return an
SVN_ERR_CANCELED error, and the second is 'are we in the process of
backing out because we received a cancelation', which causes
svn_error_clear_all to return an SVN_ERR_CANCELED error.

i can't for the life of me recall why i required these two states... i
wrote this code very late at night, and my memory of my reasoning is a
bit fuzzy. i think i saw some odd results when i was simply having
SVN_ERR unconditionally return an SVN_ERR_CANCELED error once we have
been canceled, but i forget the details.

in any event, this seems to work reaonsably well. i was able to cancel
safely out of everything i tried. the client app should probably be
checking for a SVN_ERR_CANCELED error and altering its output, since
printing out what is potentially a chain of uninteresting errors when
the user hit control-c is not really useful.

this patch is available at
http://electricjellyfish.net/garrett/code/cancel.diff since i suspect
mozilla's mailer will munge it...

note that i don't plan on actually committing this without a bunch more
work. the thread safety issues need to be considered, and a fair amount
of other things need to be cleaned up, but here's what i've got so far.

-garrett

Index: subversion/include/svn_error_codes.h
===================================================================
--- subversion/include/svn_error_codes.h
+++ subversion/include/svn_error_codes.h Thu Sep 12 19:29:06 2002
@@ -643,6 +643,10 @@
                SVN_ERR_MISC_CATEGORY_START + 12,
                "Error calling external program")

+ SVN_ERRDEF (SVN_ERR_CANCELED,
+ SVN_ERR_MISC_CATEGORY_START + 13,
+ "Operation was canceled")
+
    /* command-line client errors */

    SVN_ERRDEF (SVN_ERR_CL_ARG_PARSING_ERROR,
Index: subversion/include/svn_error.h
===================================================================
--- subversion/include/svn_error.h
+++ subversion/include/svn_error.h Thu Sep 12 22:49:39 2002
@@ -117,7 +117,7 @@
  /* Clear ERROR's pool. Note that this is likely the top-level error
     pool shared with any other errors currently extant, though usually
     only one error exists at a time, so this is not a problem. */
-void svn_error_clear_all (svn_error_t *error);
+svn_error_t *svn_error_clear_all (svn_error_t *error);

  /* Very basic default error handler: print out error stack, and quit
@@ -130,6 +130,30 @@
     which must be passed in DATA. Allocates from a subpool of POOL. */
  void svn_handle_warning (apr_pool_t *pool, void *data, const char
*fmt, ...);

+/* XXX i doubt these should go here. maybe an svn_cancel.h? */
+
+/* the next three prototypes are an implementation detail...
+ these are not the functions you are looking for...
+ move along, move along... */
+
+/* has the operation currently in progress been canceled by the client
app? */
+svn_boolean_t svn__async_canceled (void);
+
+/* generate an SVN_ERR_CANCEL error allocated from the previously
registered
+ cancelation pool */
+svn_error_t *svn__async_cancelation_error (void);
+
+/* are we currenlty cleaning up from a cancelation? */
+svn_boolean_t svn__async_cancelation_in_progress (void);
+
+/* register a pool to be used to allocate SVN_ERR_CANCEL errors. */
+void svn_async_register_cancelation_pool (apr_pool_t *pool);
+
+/* cancel the currently in progress action. */
+void svn_async_cancel (void);
+
+/* reset the global cancelation flags after a cancelation has occured. */
+void svn_async_reset (void);

  /* A statement macro for checking error return values.
     Evaluate EXPR. If it yields an error, return that error from the
@@ -151,6 +175,8 @@
      svn_error_t *svn_err__temp = (expr); \
      if (svn_err__temp) \
        return svn_err__temp; \
+ else if (svn__async_canceled ()) \
+ return svn__async_cancelation_error (); \
    } while (0)

@@ -162,6 +188,8 @@
      svn_error_t *svn_err__temp = (expr); \
      if (svn_err__temp) \
        return svn_error_quick_wrap(svn_err__temp, wrap_msg); \
+ else if (svn__async_canceled ()) \
+ return svn__async_cancelation_error (); \
    } while (0)

Index: subversion/libsvn_fs/tree.c
===================================================================
--- subversion/libsvn_fs/tree.c
+++ subversion/libsvn_fs/tree.c Thu Sep 12 21:45:12 2002
@@ -501,7 +501,7 @@
                   said it was optional, then don't return an error;
                   just put a NULL node pointer in the path. */

- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));

                if ((flags & open_path_last_optional)
                    && (! next || *next == '\0'))
@@ -2345,7 +2345,7 @@
            if (youngest_rev == youngish_rev)
              return err;
            else
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
          }
        else if (err)
          return err;
Index: subversion/libsvn_wc/log.c
===================================================================
--- subversion/libsvn_wc/log.c
+++ subversion/libsvn_wc/log.c Thu Sep 12 21:48:00 2002
@@ -626,7 +626,7 @@
    /* It's possible that locally modified files were left behind during
       the removal. That's okay; just check for this special case. */
    if (err && (err->apr_err == SVN_ERR_WC_LEFT_LOCAL_MOD))
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
    else if (err)
      return err;

Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/adm_ops.c
+++ subversion/libsvn_wc/adm_ops.c Thu Sep 12 21:47:00 2002
@@ -723,7 +723,7 @@
    err = svn_wc_entry (&orig_entry, path, TRUE, pool);
    if (err)
      {
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
        orig_entry = NULL;
      }

@@ -1550,7 +1550,7 @@

                if (err && (err->apr_err == SVN_ERR_WC_LEFT_LOCAL_MOD))
                  {
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
                    left_something = TRUE;
                  }
                else if (err)
@@ -1570,7 +1570,7 @@
                                                           destroy_wf,
subpool);
                if (err && (err->apr_err == SVN_ERR_WC_LEFT_LOCAL_MOD))
                  {
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
                    left_something = TRUE;
                  }
                else if (err)
Index: subversion/libsvn_wc/status.c
===================================================================
--- subversion/libsvn_wc/status.c
+++ subversion/libsvn_wc/status.c Thu Sep 12 21:48:38 2002
@@ -554,7 +554,7 @@
                            if (svn_err->apr_err !=
SVN_ERR_WC_NOT_DIRECTORY)
                              return svn_err;

- svn_error_clear_all (svn_err);
+ SVN_ERR (svn_error_clear_all (svn_err));
                            fullpath_entry = entry;
                          }
                      }
Index: subversion/libsvn_wc/adm_files.c
===================================================================
--- subversion/libsvn_wc/adm_files.c
+++ subversion/libsvn_wc/adm_files.c Thu Sep 12 21:46:27 2002
@@ -1020,7 +1020,7 @@

      if (err)
        {
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
          wc_exists = FALSE;
        }
      else if (wc_format > SVN_WC__VERSION)
Index: subversion/libsvn_wc/update_editor.c
===================================================================
--- subversion/libsvn_wc/update_editor.c
+++ subversion/libsvn_wc/update_editor.c Thu Sep 12 21:49:02 2002
@@ -1118,7 +1118,7 @@
          }
        else if (err)
          {
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
            hb->source = NULL; /* make sure */
          }
      }
@@ -2132,7 +2132,7 @@
    if (err || (! p_entry))
      {
        if (err)
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));

        return SVN_NO_ERROR;
      }
Index: subversion/libsvn_wc/lock.c
===================================================================
--- subversion/libsvn_wc/lock.c
+++ subversion/libsvn_wc/lock.c Thu Sep 12 21:47:35 2002
@@ -86,7 +86,7 @@
          {
            if (APR_STATUS_IS_EEXIST(err->apr_err))
              {
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
                if (wait_for <= 0)
                  break;
                wait_for--;
@@ -135,7 +135,7 @@
    if (err)
      {
        apr_status_t apr_err = err->apr_err;
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
        return apr_err;
      }
    else
@@ -198,7 +198,7 @@
    if (err)
      {
        if (err->apr_err == SVN_ERR_WC_LOCKED)
- svn_error_clear_all (err); /* Steal existing lock */
+ SVN_ERR (svn_error_clear_all (err)); /* Steal existing lock */
        else
          return err;
      }
Index: subversion/libsvn_wc/questions.c
===================================================================
--- subversion/libsvn_wc/questions.c
+++ subversion/libsvn_wc/questions.c Thu Sep 12 21:48:20 2002
@@ -74,7 +74,7 @@
               wrong at all, then for our purposes this is not a working
               copy, so return 0. */
            if (err)
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));

            *wc_format = 0;
          }
Index: subversion/libsvn_subr/svn_error.c
===================================================================
--- subversion/libsvn_subr/svn_error.c
+++ subversion/libsvn_subr/svn_error.c Thu Sep 12 23:04:45 2002
@@ -305,10 +305,16 @@
  }

-void
+svn_error_t *
  svn_error_clear_all (svn_error_t *err)
  {
- svn_pool_clear (err->pool);
+ if (svn__async_cancelation_in_progress ())
+ return svn__async_cancelation_error ();
+ else
+ {
+ svn_pool_clear (err->pool);
+ return SVN_NO_ERROR;
+ }
  }

Index: subversion/libsvn_subr/cancel.c
===================================================================
--- subversion/libsvn_subr/cancel.c
+++ subversion/libsvn_subr/cancel.c Thu Sep 12 22:36:31 2002
@@ -0,0 +1,80 @@
+/*
+ * cancel.c: support for asyncronous cancelation of subversion functions
+ *
+ * ====================================================================
+ * Copyright (c) 2000-2002 CollabNet. All rights reserved.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at http://subversion.tigris.org/license-1.html.
+ * If newer versions of this license are posted there, you may use a
+ * newer version instead, at your option.
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals. For exact contribution history, see the revision
+ * history and logs, available at http://subversion.tigris.org/.
+ * ====================================================================
+ */
+
+
+
+#include "svn_error.h"
+#include "svn_pools.h"
+
+
+/*** Code. ***/
+
+static volatile sig_atomic_t canceled = FALSE;
+
+static svn_boolean_t cancelation_in_progress = FALSE;
+
+static apr_pool_t *cancelation_pool;
+
+svn_boolean_t
+svn__async_canceled(void)
+{
+ return canceled;
+}
+
+svn_error_t *
+svn__async_cancelation_error(void)
+{
+ canceled = FALSE;
+
+ cancelation_in_progress = TRUE;
+
+ return svn_error_create (SVN_ERR_CANCELED, 0, NULL, cancelation_pool,
NULL);
+}
+
+svn_boolean_t
+svn__async_cancelation_in_progress (void)
+{
+ return cancelation_in_progress;
+}
+
+void
+svn_async_register_cancelation_pool (apr_pool_t *pool)
+{
+ cancelation_pool = pool;
+}
+
+void
+svn_async_cancel(void)
+{
+ if (! cancelation_in_progress)
+ canceled = TRUE;
+}
+
+void
+svn_async_reset(void)
+{
+ canceled = FALSE;
+
+ cancelation_in_progress = FALSE;
+}
+
+
+/*
+ * local variables:
+ * eval: (load-file "../../tools/dev/svn-dev.el")
+ * end: */
Index: subversion/libsvn_client/commit_util.c
===================================================================
--- subversion/libsvn_client/commit_util.c
+++ subversion/libsvn_client/commit_util.c Thu Sep 12 21:44:48 2002
@@ -148,7 +148,7 @@
        err = svn_wc_entries_read (&entries, path, FALSE, subpool);
        if (err)
          {
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
            entries = NULL;
          }

Index: subversion/mod_dav_svn/repos.c
===================================================================
--- subversion/mod_dav_svn/repos.c
+++ subversion/mod_dav_svn/repos.c Thu Sep 12 21:49:23 2002
@@ -1487,7 +1487,7 @@
                                  resource->pool);
    if (serr != NULL && serr->apr_err == SVN_ERR_FS_NOT_FOUND)
      {
- svn_error_clear_all(serr);
+ SVN_ERR (svn_error_clear_all(serr));
        serr = svn_fs_make_file(resource->info->root.root,
                                resource->info->repos_path,
                                resource->pool);
Index: subversion/clients/cmdline/add-cmd.c
===================================================================
--- subversion/clients/cmdline/add-cmd.c
+++ subversion/clients/cmdline/add-cmd.c Thu Sep 12 21:49:49 2002
@@ -71,7 +71,7 @@
            if (err->apr_err == SVN_ERR_ENTRY_EXISTS)
              {
                svn_handle_warning (err->pool, stderr, err->message);
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
              }
            else
              return err;
Index: subversion/clients/cmdline/resolve-cmd.c
===================================================================
--- subversion/clients/cmdline/resolve-cmd.c
+++ subversion/clients/cmdline/resolve-cmd.c Thu Sep 12 21:50:08 2002
@@ -68,7 +68,7 @@
        if (err)
          {
            svn_handle_warning (err->pool, stderr, err->message);
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
          }

        svn_pool_clear (subpool);
Index: subversion/clients/cmdline/main.c
===================================================================
--- subversion/clients/cmdline/main.c
+++ subversion/clients/cmdline/main.c Thu Sep 12 21:20:14 2002
@@ -873,6 +873,12 @@
  }

+static void
+cancel (int sig)
+{
+ svn_async_cancel();
+}
+
  
  /*** Main. ***/

@@ -913,6 +919,11 @@

    /* Create our top-level pool. */
    pool = svn_pool_create (NULL);
+
+ /* XXX where should this go? */
+ svn_async_register_cancelation_pool (pool);
+
+ signal(SIGINT, cancel);

    /* Begin processing arguments. */
    memset (&opt_state, 0, sizeof (opt_state));
Index: subversion/libsvn_repos/delta.c
===================================================================
--- subversion/libsvn_repos/delta.c
+++ subversion/libsvn_repos/delta.c Thu Sep 12 21:46:06 2002
@@ -266,7 +266,7 @@
          {
            /* Caller thinks that target still exists, but it doesn't.
               So just delete the target and go home. */
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
            SVN_ERR (delete (&c, root_baton, src_entry, pool));
            goto cleanup;
          }
@@ -282,7 +282,7 @@
          {
            /* The target has been deleted from our working copy. Add
               back a new one. */
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
            SVN_ERR (add_file_or_dir (&c, root_baton,
                                      tgt_parent_dir->data,
                                      tgt_entry->data,
Index: subversion/libsvn_ra_dav/commit.c
===================================================================
--- subversion/libsvn_ra_dav/commit.c
+++ subversion/libsvn_ra_dav/commit.c Thu Sep 12 21:45:31 2002
@@ -972,7 +972,7 @@
          {
            /* ### TODO: This is what we get if the file doesn't exist
               but an explicit not-found error might be better */
- svn_error_clear_all(err);
+ SVN_ERR (svn_error_clear_all(err));
          }
        else
          {

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Sep 13 15:17:05 2002

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