You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2010/12/10 13:58:44 UTC
svn commit: r1044334 - in /subversion/trunk/subversion: svnlook/main.c
tests/cmdline/svnlook_tests.py
Author: julianfoad
Date: Fri Dec 10 12:58:44 2010
New Revision: 1044334
URL: http://svn.apache.org/viewvc?rev=1044334&view=rev
Log:
Make svnlook detect and reject extra command-line arguments. Previously it
ignored them. In one case - svnlook lock - this changes the error message
for too few arguments from "Missing path argument" to "Missing repository
path argument" which makes it consistent with other subcommands.
Fix the tests: some of them were passing a spurious argument to svnlook.
* subversion/svnlook/main.c
(check_number_of_args): New function.
(subcommand_author, subcommand_changed, subcommand_date, subcommand_diff,
subcommand_dirschanged, subcommand_info, subcommand_log, subcommand_uuid,
subcommand_youngest): Call check_number_of_args() to check there are no
extra arguments.
(subcommand_cat, subcommand_filesize, subcommand_lock, subcommand_plist):
Call check_number_of_args() to replace a check for too few arguments.
(subcommand_history, subcommand_pget, subcommand_tree): Add a check for
too many arguments.
* subversion/tests/cmdline/svnlook_tests.py
(diff_ignore_whitespace, diff_ignore_eolstyle, diff_binary): Don't pass a
path-in-repos argument to "svnlook diff", as it doesn't want one.
(test_txn_flag): Avoid passing a spurious empty argument to "svnlook". The
hook script template was splitting the list of options with "split(' ')"
which converts an empty string into a list containing one empty string.
Use "split()" instead, so that an empty string results in an empty list.
Modified:
subversion/trunk/subversion/svnlook/main.c
subversion/trunk/subversion/tests/cmdline/svnlook_tests.py
Modified: subversion/trunk/subversion/svnlook/main.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnlook/main.c?rev=1044334&r1=1044333&r2=1044334&view=diff
==============================================================================
--- subversion/trunk/subversion/svnlook/main.c (original)
+++ subversion/trunk/subversion/svnlook/main.c Fri Dec 10 12:58:44 2010
@@ -1841,6 +1841,24 @@ warning_func(void *baton,
}
+/* Return an error if the number of arguments (excluding the repository
+ * argument) is not NUM_ARGS. NUM_ARGS must be 0 or 1. The arguments
+ * are assumed to be found in OPT_STATE->arg1 and OPT_STATE->arg2. */
+static svn_error_t *
+check_number_of_args(struct svnlook_opt_state *opt_state,
+ int num_args)
+{
+ if ((num_args == 0 && opt_state->arg1 != NULL)
+ || (num_args == 1 && opt_state->arg2 != NULL))
+ return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("Too many arguments given"));
+ if ((num_args == 1 && opt_state->arg1 == NULL))
+ return svn_error_create(SVN_ERR_CL_INSUFFICIENT_ARGS, NULL,
+ _("Missing repository path argument"));
+ return SVN_NO_ERROR;
+}
+
+
/* Factory function for the context baton. */
static svn_error_t *
get_ctxt_baton(svnlook_ctxt_t **baton_p,
@@ -1887,6 +1905,8 @@ subcommand_author(apr_getopt_t *os, void
struct svnlook_opt_state *opt_state = baton;
svnlook_ctxt_t *c;
+ SVN_ERR(check_number_of_args(opt_state, 0));
+
SVN_ERR(get_ctxt_baton(&c, opt_state, pool));
SVN_ERR(do_author(c, pool));
return SVN_NO_ERROR;
@@ -1899,10 +1919,7 @@ subcommand_cat(apr_getopt_t *os, void *b
struct svnlook_opt_state *opt_state = baton;
svnlook_ctxt_t *c;
- if (opt_state->arg1 == NULL)
- return svn_error_createf
- (SVN_ERR_CL_INSUFFICIENT_ARGS, NULL,
- _("Missing repository path argument"));
+ SVN_ERR(check_number_of_args(opt_state, 1));
SVN_ERR(get_ctxt_baton(&c, opt_state, pool));
SVN_ERR(do_cat(c, opt_state->arg1, pool));
@@ -1916,6 +1933,8 @@ subcommand_changed(apr_getopt_t *os, voi
struct svnlook_opt_state *opt_state = baton;
svnlook_ctxt_t *c;
+ SVN_ERR(check_number_of_args(opt_state, 0));
+
SVN_ERR(get_ctxt_baton(&c, opt_state, pool));
SVN_ERR(do_changed(c, pool));
return SVN_NO_ERROR;
@@ -1928,6 +1947,8 @@ subcommand_date(apr_getopt_t *os, void *
struct svnlook_opt_state *opt_state = baton;
svnlook_ctxt_t *c;
+ SVN_ERR(check_number_of_args(opt_state, 0));
+
SVN_ERR(get_ctxt_baton(&c, opt_state, pool));
SVN_ERR(do_date(c, pool));
return SVN_NO_ERROR;
@@ -1940,6 +1961,8 @@ subcommand_diff(apr_getopt_t *os, void *
struct svnlook_opt_state *opt_state = baton;
svnlook_ctxt_t *c;
+ SVN_ERR(check_number_of_args(opt_state, 0));
+
SVN_ERR(get_ctxt_baton(&c, opt_state, pool));
SVN_ERR(do_diff(c, pool));
return SVN_NO_ERROR;
@@ -1952,6 +1975,8 @@ subcommand_dirschanged(apr_getopt_t *os,
struct svnlook_opt_state *opt_state = baton;
svnlook_ctxt_t *c;
+ SVN_ERR(check_number_of_args(opt_state, 0));
+
SVN_ERR(get_ctxt_baton(&c, opt_state, pool));
SVN_ERR(do_dirs_changed(c, pool));
return SVN_NO_ERROR;
@@ -1964,10 +1989,7 @@ subcommand_filesize(apr_getopt_t *os, vo
struct svnlook_opt_state *opt_state = baton;
svnlook_ctxt_t *c;
- if (opt_state->arg1 == NULL)
- return svn_error_createf
- (SVN_ERR_CL_INSUFFICIENT_ARGS, NULL,
- _("Missing repository path argument"));
+ SVN_ERR(check_number_of_args(opt_state, 1));
SVN_ERR(get_ctxt_baton(&c, opt_state, pool));
SVN_ERR(do_filesize(c, opt_state->arg1, pool));
@@ -2012,10 +2034,11 @@ subcommand_history(apr_getopt_t *os, voi
{
struct svnlook_opt_state *opt_state = baton;
svnlook_ctxt_t *c;
- const char *path = "/";
+ const char *path = (opt_state->arg1 ? opt_state->arg1 : "/");
- if (opt_state->arg1)
- path = opt_state->arg1;
+ if (opt_state->arg2 != NULL)
+ return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("Too many arguments given"));
SVN_ERR(get_ctxt_baton(&c, opt_state, pool));
SVN_ERR(do_history(c, path, pool));
@@ -2029,18 +2052,13 @@ subcommand_lock(apr_getopt_t *os, void *
{
struct svnlook_opt_state *opt_state = baton;
svnlook_ctxt_t *c;
- const char *path;
svn_lock_t *lock;
- if (opt_state->arg1)
- path = opt_state->arg1;
- else
- return svn_error_create(SVN_ERR_CL_INSUFFICIENT_ARGS, NULL,
- _("Missing path argument"));
+ SVN_ERR(check_number_of_args(opt_state, 1));
SVN_ERR(get_ctxt_baton(&c, opt_state, pool));
- SVN_ERR(svn_fs_get_lock(&lock, c->fs, path, pool));
+ SVN_ERR(svn_fs_get_lock(&lock, c->fs, opt_state->arg1, pool));
if (lock)
{
@@ -2078,6 +2096,8 @@ subcommand_info(apr_getopt_t *os, void *
struct svnlook_opt_state *opt_state = baton;
svnlook_ctxt_t *c;
+ SVN_ERR(check_number_of_args(opt_state, 0));
+
SVN_ERR(get_ctxt_baton(&c, opt_state, pool));
SVN_ERR(do_author(c, pool));
SVN_ERR(do_date(c, pool));
@@ -2092,6 +2112,8 @@ subcommand_log(apr_getopt_t *os, void *b
struct svnlook_opt_state *opt_state = baton;
svnlook_ctxt_t *c;
+ SVN_ERR(check_number_of_args(opt_state, 0));
+
SVN_ERR(get_ctxt_baton(&c, opt_state, pool));
SVN_ERR(do_log(c, FALSE, pool));
return SVN_NO_ERROR;
@@ -2117,6 +2139,10 @@ subcommand_pget(apr_getopt_t *os, void *
(SVN_ERR_CL_INSUFFICIENT_ARGS, NULL,
_("Missing propname or repository path argument"));
}
+ if ((opt_state->revprop && opt_state->arg2 != NULL)
+ || os->ind < os->argc)
+ return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("Too many arguments given"));
SVN_ERR(get_ctxt_baton(&c, opt_state, pool));
SVN_ERR(do_pget(c, opt_state->arg1,
@@ -2131,10 +2157,7 @@ subcommand_plist(apr_getopt_t *os, void
struct svnlook_opt_state *opt_state = baton;
svnlook_ctxt_t *c;
- if (!opt_state->revprop && opt_state->arg1 == NULL)
- return svn_error_create
- (SVN_ERR_CL_INSUFFICIENT_ARGS, NULL,
- _("Missing repository path argument"));
+ SVN_ERR(check_number_of_args(opt_state, opt_state->revprop ? 0 : 1));
SVN_ERR(get_ctxt_baton(&c, opt_state, pool));
SVN_ERR(do_plist(c, opt_state->revprop ? NULL : opt_state->arg1,
@@ -2149,6 +2172,10 @@ subcommand_tree(apr_getopt_t *os, void *
struct svnlook_opt_state *opt_state = baton;
svnlook_ctxt_t *c;
+ if (opt_state->arg2 != NULL)
+ return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("Too many arguments given"));
+
SVN_ERR(get_ctxt_baton(&c, opt_state, pool));
SVN_ERR(do_tree(c, opt_state->arg1 ? opt_state->arg1 : "",
opt_state->show_ids, opt_state->full_paths,
@@ -2163,6 +2190,8 @@ subcommand_youngest(apr_getopt_t *os, vo
struct svnlook_opt_state *opt_state = baton;
svnlook_ctxt_t *c;
+ SVN_ERR(check_number_of_args(opt_state, 0));
+
SVN_ERR(get_ctxt_baton(&c, opt_state, pool));
SVN_ERR(svn_cmdline_printf(pool, "%ld\n", c->rev_id));
return SVN_NO_ERROR;
@@ -2176,6 +2205,8 @@ subcommand_uuid(apr_getopt_t *os, void *
svnlook_ctxt_t *c;
const char *uuid;
+ SVN_ERR(check_number_of_args(opt_state, 0));
+
SVN_ERR(get_ctxt_baton(&c, opt_state, pool));
SVN_ERR(svn_fs_get_uuid(c->fs, &uuid, pool));
SVN_ERR(svn_cmdline_printf(pool, "%s\n", uuid));
Modified: subversion/trunk/subversion/tests/cmdline/svnlook_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svnlook_tests.py?rev=1044334&r1=1044333&r2=1044334&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svnlook_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svnlook_tests.py Fri Dec 10 12:58:44 2010
@@ -471,14 +471,14 @@ def diff_ignore_whitespace(sbox):
# Check the output of 'svnlook diff -x --ignore-space-change' on mu.
# It should not print anything.
output = run_svnlook('diff', '-r2', '-x', '--ignore-space-change',
- repo_dir, '/A/mu')
+ repo_dir)
if output != []:
raise svntest.Failure
# Check the output of 'svnlook diff -x --ignore-all-space' on mu.
# It should not print anything.
output = run_svnlook('diff', '-r2', '-x', '--ignore-all-space',
- repo_dir, '/A/mu')
+ repo_dir)
if output != []:
raise svntest.Failure
@@ -528,7 +528,7 @@ def diff_ignore_eolstyle(sbox):
output = run_svnlook('diff', '-r', str(rev + 1), '-x',
- '--ignore-eol-style', repo_dir, '/A/mu')
+ '--ignore-eol-style', repo_dir)
rev += 1
canonical_mu_path = mu_path.replace(os.path.sep, '/')
@@ -565,7 +565,7 @@ def diff_binary(sbox):
svntest.main.run_svn(None, 'ci', '-m', 'log msg', mu_path)
# Now run 'svnlook diff' and look for the "Binary files differ" message.
- output = run_svnlook('diff', repo_dir, '/A/mu')
+ output = run_svnlook('diff', repo_dir)
if not "(Binary files differ)\n" in output:
raise svntest.Failure("No 'Binary files differ' indication in "
"'svnlook diff' output.")
@@ -637,7 +637,7 @@ def output_command(fp, cmd, opt):
return status
for (svnlook_cmd, svnlook_opt) in %s:
- output_command(fp, svnlook_cmd, svnlook_opt.split(' '))
+ output_command(fp, svnlook_cmd, svnlook_opt.split())
fp.close()"""
pre_commit_hook = svntest.main.get_pre_commit_hook_path(repo_dir)