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

[PATCH] add pool parameter to (finish|abort)_report APIs (was: Re: Another API wart: {finish,abort}_report pools)

From: Scott Collins <scc_at_mozilla.org>
Date: 2003-12-27 01:28:53 CET

Here's a log comment and patch implementing the API change suggested by
Greg Hudson. One thing this patch does _not_ do is remove the
remembered pool from the reporter baton, nor change uses of that
remembered pool. Since I could make this API change without doing
that, and since the result builds and passes all tests, I figured such
a change might be logically distinct and deserve its own patch. It
almost certainly deserves its own discussion.

Greg Hudson wrote (in part):
> and enter it as a 1.0 candidate

Does that mean I should file an issue, note the issue in this log
comment, and do some other things of which I'm not exactly sure (e.g.,
attach the patch to the issue, set some flag on the issue)? I'm happy
to do so, just tell me what or where to look to figure it out for
myself. Also, although this patch seems pretty mechanical to me, but
I'm ready to act on any feedback.

Since I've included the patch in-line, I elected not to PGP sign this
message.

Hope this helps,
__________
Scott Collins <http://ScottCollins.net/>

[[[
Add a pool parameter to the finish_report and abort_report families of
signatures.

* subversion/include/svn_repos.h
   (svn_repos_finish_report, svn_repos_abort_report): Added pool
parameter.

* subversion/include/svn_ra.h
   (svn_ra_reporter_t): Added pool parameter to the finish_report and
   abort_report members.

* subversion/libsvn_ra_local/ra_plugin.c
   (reporter_finish_report, reporter_abort_report): Added a pool
parameter and
   fixed their calls to svn_repos_finish_report and
svn_repos_abort_report to
   pass the pool.

* subversion/libsvn_repos/reporter.c
   (finish_report, svn_repos_finish_report, svn_repos_abort_report):
Added a pool
   parameter.

* subversion/libsvn_ra_svn/client.c
   (ra_svn_finish_report, ra_svn_abort_report): Added a pool parameter.

* subversion/libsvn_ra_dav/fetch.c
   (reporter_finish_report, reporter_abort_report): Added a pool
parameter.

* subversion/libsvn_wc/adm_crawler.c
* subversion/libsvn_client/export.c
* subversion/libsvn_client/diff.c
* contrib/client-side/svn-push/svn-push.c
* subversion/mod_dav_svn/update.c
* subversion/svnserve/serve.c
   Fixed all callers.

]]]

Index: subversion/include/svn_repos.h
===================================================================
--- subversion/include/svn_repos.h (revision 8089)
+++ subversion/include/svn_repos.h (working copy)
@@ -322,7 +322,8 @@
   * aborted even if the editor drive fails, so the caller does not need
   * to clean up.
   */
-svn_error_t *svn_repos_finish_report (void *report_baton);
+svn_error_t *svn_repos_finish_report (void *report_baton,
+ apr_pool_t *pool);

  /** The report-driver is bailing, so abort the fs transaction. This
@@ -330,7 +331,8 @@
   * called. No other reporting functions should be called after calling
   * this function.
   */
-svn_error_t *svn_repos_abort_report (void *report_baton);
+svn_error_t *svn_repos_abort_report (void *report_baton,
+ apr_pool_t *pool);

  /* ---------------------------------------------------------------*/
Index: subversion/include/svn_ra.h
===================================================================
--- subversion/include/svn_ra.h (revision 8089)
+++ subversion/include/svn_ra.h (working copy)
@@ -163,12 +163,14 @@
     * or files not explicitly `set' above are assumed to be at the
     * baseline revision originally passed into @c do_update().
     */
- svn_error_t *(*finish_report) (void *report_baton);
+ svn_error_t *(*finish_report) (void *report_baton,
+ apr_pool_t *pool);

    /** If an error occurs during a report, this routine should cause the
     * filesystem transaction to be aborted & cleaned up.
     */
- svn_error_t *(*abort_report) (void *report_baton);
+ svn_error_t *(*abort_report) (void *report_baton,
+ apr_pool_t *pool);

  } svn_ra_reporter_t;

Index: subversion/libsvn_wc/adm_crawler.c
===================================================================
--- subversion/libsvn_wc/adm_crawler.c (revision 8089)
+++ subversion/libsvn_wc/adm_crawler.c (working copy)
@@ -563,14 +563,14 @@
      }

    /* Finish the report, which causes the update editor to be driven. */
- SVN_ERR (reporter->finish_report (report_baton));
+ SVN_ERR (reporter->finish_report (report_baton, pool));

   abort_report:
    if (err)
      {
        /* Clean up the fs transaction. */
        svn_error_t *fserr;
- if ((fserr = reporter->abort_report (report_baton)))
+ if ((fserr = reporter->abort_report (report_baton, pool)))
          {
            fserr = svn_error_quick_wrap (fserr, "Error aborting
report");
            svn_error_compose (err, fserr);
Index: subversion/libsvn_ra_local/ra_plugin.c
===================================================================
--- subversion/libsvn_ra_local/ra_plugin.c (revision 8089)
+++ subversion/libsvn_ra_local/ra_plugin.c (working copy)
@@ -101,18 +101,20 @@

  static svn_error_t *
-reporter_finish_report (void *reporter_baton)
+reporter_finish_report (void *reporter_baton,
+ apr_pool_t *pool)
  {
    reporter_baton_t *rbaton = reporter_baton;
- return svn_repos_finish_report (rbaton->report_baton);
+ return svn_repos_finish_report (rbaton->report_baton, pool);
  }

  static svn_error_t *
-reporter_abort_report (void *reporter_baton)
+reporter_abort_report (void *reporter_baton,
+ apr_pool_t *pool)
  {
    reporter_baton_t *rbaton = reporter_baton;
- return svn_repos_abort_report (rbaton->report_baton);
+ return svn_repos_abort_report (rbaton->report_baton, pool);
  }

Index: subversion/libsvn_client/export.c
===================================================================
--- subversion/libsvn_client/export.c (revision 8089)
+++ subversion/libsvn_client/export.c (working copy)
@@ -697,7 +697,7 @@
                                     TRUE, /* "help, my dir is empty!" */
                                     pool));

- SVN_ERR (reporter->finish_report (report_baton));
+ SVN_ERR (reporter->finish_report (report_baton, pool));

        /* Special case: Due to our sly export/checkout method of
         * updating an empty directory, no target will have been created
Index: subversion/libsvn_client/diff.c
===================================================================
--- subversion/libsvn_client/diff.c (revision 8089)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -1274,7 +1274,7 @@

    SVN_ERR (reporter->set_path (report_baton, "", start_revnum, FALSE,
pool));

- SVN_ERR (reporter->finish_report (report_baton));
+ SVN_ERR (reporter->finish_report (report_baton, pool));

    return SVN_NO_ERROR;
  }
@@ -1641,7 +1641,7 @@

    /* Drive the reporter; do the diff. */
    SVN_ERR (reporter->set_path (report_baton, "", rev1, FALSE, pool));
- SVN_ERR (reporter->finish_report (report_baton));
+ SVN_ERR (reporter->finish_report (report_baton, pool));

    return SVN_NO_ERROR;
  }
Index: subversion/mod_dav_svn/update.c
===================================================================
--- subversion/mod_dav_svn/update.c (revision 8089)
+++ subversion/mod_dav_svn/update.c (working copy)
@@ -1274,7 +1274,7 @@
              /* we require the `rev' attribute for this to make sense */
              if (! SVN_IS_VALID_REVNUM (rev))
                {
- svn_error_clear(svn_repos_abort_report(rbaton));
+ svn_error_clear(svn_repos_abort_report(rbaton,
resource->pool));
                  serr = svn_error_create (SVN_ERR_XML_ATTRIB_NOT_FOUND,
                                           NULL, "Missing XML attribute:
rev");
                  return dav_svn_convert_err(serr,
HTTP_INTERNAL_SERVER_ERROR,
@@ -1294,7 +1294,7 @@
                                           start_empty, resource->pool);
              if (serr != NULL)
                {
- svn_error_clear(svn_repos_abort_report(rbaton));
+ svn_error_clear(svn_repos_abort_report(rbaton,
resource->pool));
                  return dav_svn_convert_err(serr,
HTTP_INTERNAL_SERVER_ERROR,
                                             "A failure occurred while "
                                             "recording one of the items
of "
@@ -1326,7 +1326,7 @@
              serr = svn_repos_delete_path(rbaton, path, resource->pool);
              if (serr != NULL)
                {
- svn_error_clear(svn_repos_abort_report(rbaton));
+ svn_error_clear(svn_repos_abort_report(rbaton,
resource->pool));
                  return dav_svn_convert_err(serr,
HTTP_INTERNAL_SERVER_ERROR,
                                             "A failure occurred while "
                                             "recording one of the
(missing) "
@@ -1338,7 +1338,7 @@

    /* this will complete the report, and then drive our editor to
generate
       the response to the client. */
- serr = svn_repos_finish_report(rbaton);
+ serr = svn_repos_finish_report(rbaton, resource->pool);
    if (serr)
      return dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
                                 "A failure occurred while "
@@ -1408,7 +1408,7 @@
       resource-walker... */
    if (serr != NULL)
      {
- svn_error_clear(svn_repos_abort_report(rbaton));
+ svn_error_clear(svn_repos_abort_report(rbaton, resource->pool));
        return dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
                                   "A failure occurred during the
completion "
                                   "and response generation for the
update "
Index: subversion/libsvn_repos/reporter.c
===================================================================
--- subversion/libsvn_repos/reporter.c (revision 8089)
+++ subversion/libsvn_repos/reporter.c (working copy)
@@ -430,7 +430,8 @@
   * any txns being aborted.
   */
  static svn_error_t *
-finish_report (void *report_baton)
+finish_report (void *report_baton,
+ apr_pool_t *pool)
  {
    svn_fs_root_t *root1, *root2;
    report_baton_t *rbaton = report_baton;
@@ -491,10 +492,11 @@
   * finish_report returns an error.
   */
  svn_error_t *
-svn_repos_finish_report (void *report_baton)
+svn_repos_finish_report (void *report_baton,
+ apr_pool_t *pool)
  {
- svn_error_t *err1 = finish_report (report_baton);
- svn_error_t *err2 = svn_repos_abort_report (report_baton);
+ svn_error_t *err1 = finish_report (report_baton, pool);
+ svn_error_t *err2 = svn_repos_abort_report (report_baton, pool);
    if (err1)
      {
        svn_error_clear (err2);
@@ -505,7 +507,8 @@

  svn_error_t *
-svn_repos_abort_report (void *report_baton)
+svn_repos_abort_report (void *report_baton,
+ apr_pool_t *pool)
  {
    report_baton_t *rbaton = report_baton;

Index: subversion/libsvn_ra_svn/client.c
===================================================================
--- subversion/libsvn_ra_svn/client.c (revision 8089)
+++ subversion/libsvn_ra_svn/client.c (working copy)
@@ -345,7 +345,8 @@
    return SVN_NO_ERROR;
  }

-static svn_error_t *ra_svn_finish_report(void *baton)
+static svn_error_t *ra_svn_finish_report(void *baton,
+ apr_pool_t *pool)
  {
    ra_svn_reporter_baton_t *b = baton;

@@ -357,7 +358,8 @@
    return SVN_NO_ERROR;
  }

-static svn_error_t *ra_svn_abort_report(void *baton)
+static svn_error_t *ra_svn_abort_report(void *baton,
+ apr_pool_t *pool)
  {
    ra_svn_reporter_baton_t *b = baton;

Index: subversion/libsvn_ra_dav/fetch.c
===================================================================
--- subversion/libsvn_ra_dav/fetch.c (revision 8089)
+++ subversion/libsvn_ra_dav/fetch.c (working copy)
@@ -2239,7 +2239,8 @@
  }

-static svn_error_t * reporter_abort_report(void *report_baton)
+static svn_error_t * reporter_abort_report(void *report_baton,
+ apr_pool_t *pool)
  {
    report_baton_t *rb = report_baton;

@@ -2249,7 +2250,8 @@
  }

-static svn_error_t * reporter_finish_report(void *report_baton)
+static svn_error_t * reporter_finish_report(void *report_baton,
+ apr_pool_t *pool)
  {
    report_baton_t *rb = report_baton;
    svn_error_t *err;
Index: subversion/svnserve/serve.c
===================================================================
--- subversion/svnserve/serve.c (revision 8089)
+++ subversion/svnserve/serve.c (working copy)
@@ -290,7 +290,7 @@
    /* No arguments to parse. */
    SVN_ERR(trivial_auth_request(conn, pool, b->sb));
    if (!b->err)
- b->err = svn_repos_finish_report(b->report_baton);
+ b->err = svn_repos_finish_report(b->report_baton, pool);
    return SVN_NO_ERROR;
  }

@@ -300,7 +300,7 @@
    report_driver_baton_t *b = baton;

    /* No arguments to parse. */
- svn_error_clear(svn_repos_abort_report(b->report_baton));
+ svn_error_clear(svn_repos_abort_report(b->report_baton, pool));
    return SVN_NO_ERROR;
  }

Index: contrib/client-side/svn-push/svn-push.c
===================================================================
--- contrib/client-side/svn-push/svn-push.c (revision 8089)
+++ contrib/client-side/svn-push/svn-push.c (working copy)
@@ -135,7 +135,7 @@

    SVN_ERR (reporter->set_path (report_baton, "", start_rev, 0, pool));

- SVN_ERR (reporter->finish_report (report_baton));
+ SVN_ERR (reporter->finish_report (report_baton, pool));

    return SVN_NO_ERROR;
  }

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Dec 27 01:58:11 2003

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.