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

Re: [PATCH] svn_opt_args_to_target_array2 continued

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2004-11-12 00:57:41 CET

That patch exacerbated a problem that was already there to a lesser extent: it
was wrongly using native-encoded paths (from os->argv[]) before they had been
converted to UTF-8. This version fixes that.

- Julian

In "svn diff", avoid using a deprecated function and fix an encoding bug.

* subversion/clients/cmdline/diff-cmd.c
  (parse_path): New helper function.
  (svn_cl__diff): Simplify the code for argument parsing, using a helper,
    and avoid using the recently deprecated svn_opt_args_to_target_array.
    Fix encoding bug: was passing a native-encoded path to svn_path_is_url.

Index: subversion/clients/cmdline/diff-cmd.c
===================================================================
--- subversion/clients/cmdline/diff-cmd.c (revision 11829)
+++ subversion/clients/cmdline/diff-cmd.c (working copy)
@@ -38,6 +38,24 @@
 
 /*** Code. ***/
 
+/* Parse PATH of the form "path[@rev]" into TRUEPATH and *REV,
+ overwriting *REV only if "@rev" was present. */
+static svn_error_t *
+parse_path (svn_opt_revision_t *rev,
+ const char **truepath,
+ const char *path,
+ apr_pool_t *pool)
+{
+ svn_opt_revision_t new_rev;
+ const char *new_path;
+
+ SVN_ERR (svn_opt_parse_path (&new_rev, &new_path, path, pool));
+ *truepath = svn_path_canonicalize (new_path, pool);
+ if (new_rev.kind != svn_opt_revision_unspecified)
+ *rev = new_rev;
+ return SVN_NO_ERROR;
+}
+
 /* An svn_opt_subcommand_t to handle the 'diff' command.
    This implements the `svn_opt_subcommand_t' interface. */
 svn_error_t *
@@ -68,22 +86,22 @@
   if ((status = apr_file_open_stderr (&errfile, pool)))
     return svn_error_wrap_apr (status, _("Can't open stderr"));
 
+ SVN_ERR (svn_opt_args_to_target_array2 (&targets, os,
+ opt_state->targets, pool));
+
   if (! opt_state->old_target && ! opt_state->new_target
- && (os->argc - os->ind == 2)
- && svn_path_is_url (os->argv[os->ind])
- && svn_path_is_url (os->argv[os->ind + 1])
+ && (targets->nelts == 2)
+ && svn_path_is_url (APR_ARRAY_IDX (targets, 0, const char *))
+ && svn_path_is_url (APR_ARRAY_IDX (targets, 1, const char *))
       && opt_state->start_revision.kind == svn_opt_revision_unspecified
       && opt_state->end_revision.kind == svn_opt_revision_unspecified)
     {
       /* The 'svn diff OLD_URL[@OLDREV] NEW_URL[@NEWREV]' case matches. */
- SVN_ERR (svn_opt_args_to_target_array (&targets, os,
- opt_state->targets,
- &(opt_state->start_revision),
- &(opt_state->end_revision),
- TRUE, /* extract @revs */ pool));
 
- old_target = APR_ARRAY_IDX (targets, 0, const char *);
- new_target = APR_ARRAY_IDX (targets, 1, const char *);
+ SVN_ERR (parse_path (&opt_state->start_revision, &old_target,
+ APR_ARRAY_IDX (targets, 0, const char *), pool));
+ SVN_ERR (parse_path (&opt_state->end_revision, &new_target,
+ APR_ARRAY_IDX (targets, 1, const char *), pool));
       targets->nelts = 0;
       
       if (opt_state->start_revision.kind == svn_opt_revision_unspecified)
@@ -93,28 +111,17 @@
     }
   else if (opt_state->old_target)
     {
- apr_array_header_t *tmp, *tmp2;
-
       /* The 'svn diff --old=OLD[@OLDREV] [--new=NEW[@NEWREV]]
          [PATH...]' case matches. */
 
- SVN_ERR (svn_opt_args_to_target_array2 (&targets, os,
- opt_state->targets, pool));
-
- tmp = apr_array_make (pool, 2, sizeof (const char *));
- APR_ARRAY_PUSH (tmp, const char *) = (opt_state->old_target);
- APR_ARRAY_PUSH (tmp, const char *) = (opt_state->new_target
- ? opt_state->new_target
- : APR_ARRAY_IDX (tmp, 0,
- const char *));
-
- SVN_ERR (svn_opt_args_to_target_array (&tmp2, os, tmp,
- &(opt_state->start_revision),
- &(opt_state->end_revision),
- TRUE, /* extract @revs */ pool));
+ SVN_ERR (parse_path (&opt_state->start_revision, &old_target,
+ opt_state->old_target, pool));
 
- old_target = APR_ARRAY_IDX (tmp2, 0, const char *);
- new_target = APR_ARRAY_IDX (tmp2, 1, const char *);
+ SVN_ERR (parse_path (&opt_state->end_revision, &new_target,
+ opt_state->new_target
+ ? opt_state->new_target
+ : opt_state->old_target,
+ pool));
 
       if (opt_state->start_revision.kind == svn_opt_revision_unspecified)
         opt_state->start_revision.kind = svn_path_is_url (old_target)
@@ -126,29 +133,17 @@
     }
   else
     {
- apr_array_header_t *tmp, *tmp2;
       svn_boolean_t working_copy_present = FALSE, url_present = FALSE;
       
       /* The 'svn diff [-r M[:N]] [TARGET[@REV]...]' case matches. */
 
       /* Here each target is a pegged object. Find out the starting
          and ending paths for each target. */
- SVN_ERR (svn_opt_args_to_target_array2 (&targets, os,
- opt_state->targets, pool));
 
       svn_opt_push_implicit_dot_target (targets, pool);
 
- tmp = apr_array_make (pool, 2, sizeof (const char *));
- APR_ARRAY_PUSH (tmp, const char *) = ".";
- APR_ARRAY_PUSH (tmp, const char *) = ".";
-
- SVN_ERR (svn_opt_args_to_target_array (&tmp2, os, tmp,
- &(opt_state->start_revision),
- &(opt_state->end_revision),
- TRUE, /* extract @revs */ pool));
-
- old_target = APR_ARRAY_IDX (tmp2, 0, const char *);
- new_target = APR_ARRAY_IDX (tmp2, 1, const char *);
+ old_target = "";
+ new_target = "";
 
       /* Check to see if at least one of our paths is a working copy
          path. */

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Nov 12 00:58:11 2004

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.