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

"serve.c" and "text_deltas" parameter of svn_repos_begin_report

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2003-02-03 23:36:43 CET

In svnserve/serve.c the two functions "switch_cmd" and "diff" are
identical except that one passes TRUE and the other FALSE to the
"text_deltas" parameter of svn_repos_begin_report. The functions are 50
lines long - not terribly complex but definitely worth factoring out to
make the similarity obvious. Of course they might not remain this
similar in future, but at present they are. A suggested factoring is
given in the first diff below. Is this appropriate?

Closely related, and therefore in this same message:

I think the comment for the "text_deltas" flag has a typo that could be
fixed by removing two spurious words, as in the second diff below. But
maybe there is more to it. I don't quite understand how this single
flag makes the whole difference between "svn diff" and "svn switch".
Could someone who knows please verify that "false" just means "don't
generate text deltas"; if it means "do something else instead", then the
comment ought to say what it does instead.

- Julian

* subversion/svnserve/serve.c:

   Factor out the common body of functions "switch_cmd" and "diff".

* subversion/include/svn_repos.h:

   Fix typo in comment.

Index: subversion/svnserve/serve.c
===================================================================
--- subversion/svnserve/serve.c (revision 4704)
+++ subversion/svnserve/serve.c (working copy)
@@ -571,13 +571,16 @@
    return handle_report(conn, pool, b->repos_url, report_baton);
  }

-static svn_error_t *switch_cmd(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
- apr_array_header_t *params, void *baton)
+/* Presently the implementations of "switch" and "diff" are so similar that
+ * they share this code. They need not continue to be so similar in
future. */
+static svn_error_t *switch_or_diff(svn_ra_svn_conn_t *conn, apr_pool_t
*pool,
+ apr_array_header_t *params, void *baton,
+ svn_boolean_t do_switch)
  {
    server_baton_t *b = baton;
    svn_revnum_t rev;
    const char *target;
- const char *switch_url, *switch_path;
+ const char *other_url, *other_path;
    svn_boolean_t recurse;
    const svn_delta_editor_t *editor;
    void *edit_baton, *report_baton;
@@ -586,31 +589,31 @@

    /* Parse the arguments. */
    SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "[r]cbc", &rev, &target,
- &recurse, &switch_url));
+ &recurse, &other_url));
    if (svn_path_is_empty(target))
      target = NULL; /* ### Compatibility hack, shouldn't be needed */
    if (!SVN_IS_VALID_REVNUM(rev))
      SVN_CMD_ERR(svn_fs_youngest_rev(&rev, b->fs, pool));

- /* Verify that switch_url is in the same repository and get its fs
path. */
- switch_url = svn_path_uri_decode(switch_url, pool);
+ /* Verify that other_url is in the same repository and get its fs
path. */
+ other_url = svn_path_uri_decode(other_url, pool);
    len = strlen(b->repos_url);
- if (strncmp(switch_url, b->repos_url, len) != 0)
+ if (strncmp(other_url, b->repos_url, len) != 0)
      {
        err = svn_error_createf (SVN_ERR_RA_ILLEGAL_URL, NULL,
                                 "'%s'\n"
                                 "is not the same repository as\n"
- "'%s'", switch_url, b->repos_url);
+ "'%s'", other_url, b->repos_url);
        /* Wrap error so that it gets reported back to the client. */
        return svn_error_create(SVN_ERR_RA_SVN_CMD_ERR, err, NULL);
      }
- switch_path = switch_url + len;
+ other_path = other_url + len;

    /* Make an svn_repos report baton. Tell it to drive the network editor
     * when the report is complete. */
    svn_ra_svn_get_editor(&editor, &edit_baton, conn, pool, NULL, NULL);
    SVN_CMD_ERR(svn_repos_begin_report(&report_baton, rev, b->user,
b->repos,
- b->fs_path, target, switch_path, TRUE,
+ b->fs_path, target, other_path,
do_switch,
                                       recurse, editor, edit_baton, pool));

    /* Write an empty command-reponse, telling the client to start
reporting. */
@@ -621,6 +624,18 @@
    return handle_report(conn, pool, b->repos_url, report_baton);
  }

+static svn_error_t *switch_cmd(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
+ apr_array_header_t *params, void *baton)
+{
+ return switch_or_diff(conn, pool, params, baton, TRUE);
+}
+
+static svn_error_t *diff(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
+ apr_array_header_t *params, void *baton)
+{
+ return switch_or_diff(conn, pool, params, baton, FALSE);
+}
+
  static svn_error_t *status(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
                             apr_array_header_t *params, void *baton)
  {
@@ -652,56 +667,6 @@
    return handle_report(conn, pool, b->repos_url, report_baton);
  }

-static svn_error_t *diff(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
- apr_array_header_t *params, void *baton)
-{
- server_baton_t *b = baton;
- svn_revnum_t rev;
- const char *target, *versus_url, *versus_path;
- svn_boolean_t recurse;
- const svn_delta_editor_t *editor;
- void *edit_baton, *report_baton;
-
- int len;
- svn_error_t *err;
-
- /* Parse the arguments. */
- SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "[r]cbc", &rev, &target,
- &recurse, &versus_url));
- if (svn_path_is_empty(target))
- target = NULL; /* ### Compatibility hack, shouldn't be needed */
- if (!SVN_IS_VALID_REVNUM(rev))
- SVN_CMD_ERR(svn_fs_youngest_rev(&rev, b->fs, pool));
-
- /* Verify that versus_url is in the same repository and get its fs
path. */
- versus_url = svn_path_uri_decode(versus_url, pool);
- len = strlen(b->repos_url);
- if (strncmp(versus_url, b->repos_url, len) != 0)
- {
- err = svn_error_createf (SVN_ERR_RA_ILLEGAL_URL, NULL,
- "'%s'\n"
- "is not the same repository as\n"
- "'%s'", versus_url, b->repos_url);
- /* Wrap error so that it gets reported back to the client. */
- return svn_error_create(SVN_ERR_RA_SVN_CMD_ERR, err, NULL);
- }
- versus_path = versus_url + len;
-
- /* Make an svn_repos report baton. Tell it to drive the network editor
- * when the report is complete. */
- svn_ra_svn_get_editor(&editor, &edit_baton, conn, pool, NULL, NULL);
- SVN_CMD_ERR(svn_repos_begin_report(&report_baton, rev, b->user, b->repos,
- b->fs_path, target, versus_path,
FALSE,
- recurse, editor, edit_baton, pool));
-
- /* Write an empty command-reponse, telling the client to start
reporting. */
- SVN_ERR(svn_ra_svn_write_cmd_response(conn, pool, ""));
-
- /* Handle the client's report; when it's done, svn_repos will drive the
- * network editor with the diff. */
- return handle_report(conn, pool, b->repos_url, report_baton);
-}
-
  /* Send a log entry to the client. */
  static svn_error_t *log_receiver(void *baton, apr_hash_t *changed_paths,
                                   svn_revnum_t rev, const char *author,
Index: subversion/include/svn_repos.h
===================================================================
--- subversion/include/svn_repos.h (revision 4704)
+++ subversion/include/svn_repos.h (working copy)
@@ -182,7 +182,7 @@
   * transform the reported heirarchy to revision @a revnum, preserving the
   * reported heirarchy.
   *
- * @a text_deltas instructs the driver of the @a editor to enable to
disable
+ * @a text_deltas instructs the driver of the @a editor to enable
   * the generation of text deltas.
   *
   * @a recurse instructs the driver of the @a editor to send a recursive

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Feb 3 23:34:15 2003

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