Check min and max num targets in client args patch.

From: Mo DeJong <mdejong_at_cygnus.com>
Date: 2001-03-06 18:49:24 CET

Hi all.

I noticed some problems with the way the command line
client was checking the user's input before passing
commands to the actual function. For example, a
call to `svn delete` should notice that at least
1 argument is required and print the help text
for the delete subcommand. The current code passed
the args to the delete subcommand anyway and that
printed the main usage text instead of the delete
help text. The following patch fixes the problem
by adding a min and max number of targets field
to the command struct. Main then checks to see
if the number of arguments the user passed
satisfies the (0 to N) or (1 to N) requirement.
I also have a patch that removes the num
targets check in the functions, but that can
wait until after this patch.

2001-03-06 Mo DeJong <mdejong@redhat.com>

        subversion/client/cl.h: Add svn_cl__num_targets
        enum, rename svn_cl__cmd_desc_t num_args field
        to num_non_target_args, add min_num_target_args
        and max_num_target_args to svn_cl__cmd_desc_t.
        subversion/client/main.c (main): Init min and
        max fields in svn_cl__cmd_desc_t command table.
        Use num_non_target_args instead of num_args.
        If the number of arguments passed by the user
        does not satisfy the min or max requirements
        of the command, print help for that subcommand.

P.S.
I am also attaching this patch in case it gets hosed
by my main client.

Mo DeJong
Red Hat Inc

Index: subversion/client/cl.h
===================================================================
RCS file: /cvs/subversion/subversion/client/cl.h,v
retrieving revision 1.34
diff -u -r1.34 cl.h
--- subversion/client/cl.h 2001/03/04 18:36:00 1.34
+++ subversion/client/cl.h 2001/03/06 17:17:48
@@ -86,6 +88,16 @@
                                            apr_pool_t *pool);

+
+/* The min and max number of target arguments supported by a command */
+
+enum svn_cl__num_targets {
+ svn_cl__target_args_N = -1,
+ svn_cl__target_args_ZERO = 0,
+ svn_cl__target_args_ONE = 1
+};
+
+
/* One element of the command dispatch table. */
typedef struct svn_cl__cmd_desc_t
{
@@ -112,11 +124,14 @@
   /* The number of non-filename arguments the command takes. (e.g. 2
    * for propset, 1 for propget, 0 for most other commands). -1 means
    * "just give me all of the arguments" */
- int num_args;
+ int num_non_target_args;

   /* A brief string describing this command, for usage messages. */
   const char *help;

+ enum svn_cl__num_targets min_num_target_args; /* ZERO or ONE */
+ enum svn_cl__num_targets max_num_target_args; /* ZERO or N */
+
} svn_cl__cmd_desc_t;

Index: subversion/client/main.c
===================================================================
RCS file: /cvs/subversion/subversion/client/main.c,v
retrieving revision 1.62
diff -u -r1.62 main.c
--- subversion/client/main.c 2001/03/04 18:23:38 1.62
+++ subversion/client/main.c 2001/03/06 17:17:48
@@ -52,32 +52,37 @@
{
   { "add", FALSE, svn_cl__add_command, svn_cl__add, 0,
     "Add new files and directories to version control.\n"
- "usage: add [TARGETS]\n" },
- { "ad", TRUE, 0, NULL, 0, NULL },
- { "new", TRUE, 0, NULL, 0, NULL },
+ "usage: add [TARGETS]\n",
+ svn_cl__target_args_ONE, svn_cl__target_args_N },
+ { "ad", TRUE, 0, NULL, 0, NULL, 0, 0 },
+ { "new", TRUE, 0, NULL, 0, NULL, 0, 0 },

   { "checkout", FALSE, svn_cl__checkout_command, svn_cl__checkout, 0,
     "Check out a working directory from a repository.\n"
- "usage: checkout REPOSPATH\n" },
- { "co", TRUE, 0, NULL, 0, NULL },
+ "usage: checkout REPOSPATH\n", /* REPOSPATH passed via op_state not targets */
+ svn_cl__target_args_ZERO, svn_cl__target_args_ZERO },
+ { "co", TRUE, 0, NULL, 0, NULL, 0, 0 },

   { "commit", FALSE, svn_cl__commit_command, svn_cl__commit, 0,
     "Commit changes from your working copy to the repository.\n"
- "usage: commit [TARGETS]\n" },
- { "ci", TRUE, 0, NULL, 0, NULL },
+ "usage: commit [TARGETS]\n",
+ svn_cl__target_args_ZERO, svn_cl__target_args_N },
+ { "ci", TRUE, 0, NULL, 0, NULL, 0, 0 },

   { "delete", FALSE, svn_cl__delete_command, svn_cl__delete, 0,
     "Remove files and directories from version control.\n"
- "usage: delete [TARGETS]\n" },
- { "del", TRUE, 0, NULL, 0, NULL },
- { "remove", TRUE, 0, NULL, 0, NULL },
- { "rm", TRUE, 0, NULL, 0, NULL },
+ "usage: delete [TARGETS]\n",
+ svn_cl__target_args_ONE, svn_cl__target_args_N },
+ { "del", TRUE, 0, NULL, 0, NULL, 0, 0 },
+ { "remove", TRUE, 0, NULL, 0, NULL, 0, 0 },
+ { "rm", TRUE, 0, NULL, 0, NULL, 0, 0 },

   { "help", FALSE, svn_cl__help_command, svn_cl__help, 0,
     "Display this usage message.\n"
- "usage: help [SUBCOMMAND1 [SUBCOMMAND2] ...]\n" },
- { "?", TRUE, 0, NULL, 0, NULL },
- { "h", TRUE, 0, NULL, 0, NULL },
+ "usage: help [SUBCOMMAND1 [SUBCOMMAND2] ...]\n",
+ svn_cl__target_args_ZERO, svn_cl__target_args_N },
+ { "?", TRUE, 0, NULL, 0, NULL, 0, 0 },
+ { "h", TRUE, 0, NULL, 0, NULL, 0, 0 },
   /* We need to support "--help", "-?", and all that good stuff, of
      course. But those options, since unknown, will result in the
      help message being printed out anyway, so there's no need to
@@ -85,39 +90,46 @@

   { "proplist", FALSE, svn_cl__proplist_command, svn_cl__proplist, 0,
     "List all properties for given files and directories.\n"
- "usage: proplist [TARGETS]\n" },
- { "plist", TRUE, 0, NULL, 0, NULL },
- { "pl", TRUE, 0, NULL, 0, NULL },
+ "usage: proplist [TARGETS]\n",
+ svn_cl__target_args_ZERO, svn_cl__target_args_N },
+ { "plist", TRUE, 0, NULL, 0, NULL, 0, 0 },
+ { "pl", TRUE, 0, NULL, 0, NULL, 0, 0 },

   { "propget", FALSE, svn_cl__propget_command, svn_cl__propget, 1,
     "Get the value of property PROPNAME on files and directories.\n"
- "usage: propget PROPNAME [TARGETS]\n" },
- { "pget", TRUE, 0, NULL, 1, NULL },
- { "pg", TRUE, 0, NULL, 1, NULL },
+ "usage: propget PROPNAME [TARGETS]\n",
+ svn_cl__target_args_ZERO, svn_cl__target_args_N },
+ { "pget", TRUE, 0, NULL, 1, NULL, 0, 0 },
+ { "pg", TRUE, 0, NULL, 1, NULL, 0, 0 },

   { "propset", FALSE, svn_cl__propset_command, svn_cl__propset, 2,
     "Set property PROPNAME to PROPVAL on the named files and directories.\n"
     "usage: propset PROPNAME [PROPVAL | --valfile VALFILE] "
- "[TARGET1 [TARGET2] ...]\n"},
- { "pset", TRUE, 0, NULL, 2, NULL },
- { "ps", TRUE, 0, NULL, 2, NULL },
+ "[TARGET1 [TARGET2] ...]\n",
+ svn_cl__target_args_ONE, svn_cl__target_args_N }, /* FIXME 0 args allowed ?? */
+ { "pset", TRUE, 0, NULL, 2, NULL, 0, 0 },
+ { "ps", TRUE, 0, NULL, 2, NULL, 0, 0 },

   { "status", FALSE, svn_cl__status_command, svn_cl__status, 0,
     "Print the status of working copy files and directories.\n"
- "usage: status [TARGETS]\n" },
- { "stat", TRUE, 0, NULL, 0, NULL },
- { "st", TRUE, 0, NULL, 0, NULL },
+ "usage: status [TARGETS]\n",
+ svn_cl__target_args_ZERO, svn_cl__target_args_N },
+ { "stat", TRUE, 0, NULL, 0, NULL, 0, 0 },
+ { "st", TRUE, 0, NULL, 0, NULL, 0, 0 },

   { "diff", FALSE, svn_cl__diff_command, svn_cl__diff, 0,
     "Display local file changes as contextual diffs.\n"
- "usage: diff [TARGETS]\n" },
- { "df", TRUE, 0, NULL, 0, NULL },
+ "usage: diff [TARGETS]\n",
+ svn_cl__target_args_ZERO, svn_cl__target_args_N },
+ { "df", TRUE, 0, NULL, 0, NULL, 0, 0 },
+ { "di", TRUE, 0, NULL, 0, NULL, 0, 0 },

   { "update", FALSE, svn_cl__update_command, svn_cl__update, 0,
     "Bring changes from the repository into the working copy.\n"
- "usage: update [TARGETS]\n" },
- { "up", TRUE, 0, NULL, 0, NULL },
- { NULL, FALSE, 0, NULL, 0, NULL }
+ "usage: update [TARGETS]\n",
+ svn_cl__target_args_ZERO, svn_cl__target_args_N },
+ { "up", TRUE, 0, NULL, 0, NULL, 0, 0 },
+ { NULL, FALSE, 0, NULL, 0, NULL, 0, 0 }
};

@@ -358,16 +370,17 @@
    * addition. */
   opt_state.args = apr_array_make (pool, 0, sizeof (svn_string_t *));

- if (subcommand->num_args > 0) {
+ if (subcommand->num_non_target_args > 0) {
     const char *this_arg;
     int i;
- /* loop for num_args and add each arg to the args array */
- for (i = 0; i < subcommand->num_args; i++) {
+ /* loop for num_non_target_args and add each arg to the args array */
+ for (i = 0; i < subcommand->num_non_target_args; i++) {
       if (os->ind >= os->argc) {
         const char *plural = "s";
         fprintf (stderr, "ERROR: The %s command requires %i argument%s\n",
- subcommand->name, subcommand->num_args,
- (subcommand->num_args == 1) ? "" : plural);
+ subcommand->name,
+ subcommand->num_non_target_args,
+ (subcommand->num_non_target_args == 1) ? "" : plural);
         fprintf (stderr, "Help for %s:\n%s", subcommand->name, subcommand->help);
         /* svn_cl__help (NULL, targets, pool); */
         /* TODO Do we have to print out the WHOLE help thing every
@@ -381,8 +394,8 @@

     }
   }
- /* greedily suck up all args if num_args is a negative number. */
- else if (subcommand->num_args == -1) {
+ /* greedily suck up all args if num_non_target_args is a negative number. */
+ else if (subcommand->num_non_target_args == -1) {
     /* TODO. This is currently not used by any subcommand, so I'm not
        writing it right now. -Fitz */
     fprintf (stderr, "unimplemented function. -1 args not written yet\n");
@@ -390,12 +403,37 @@
     return EXIT_FAILURE;
   }

- /* Do the regular arguments (target files and target dirs). */
- for (; os->ind < os->argc; os->ind++)
+ /* Do the regular arguments (target files and target dirs) */
+
+ /* Each commands knows the min and max number of target *
+ * args that it accepts. If the min is 1 and the user *
+ * passes 0 arguments, print help for cmd. If the user *
+ * passed more arguments than the max and the max is not *
+ * N, then print help for cmd. */
+
+ if ((subcommand->min_num_target_args == svn_cl__target_args_ONE
+ && (os->argc - 2) == 0) ||
+ (subcommand->max_num_target_args != svn_cl__target_args_N
+ && (os->argc - 2) > subcommand->max_num_target_args))
     {
- const char *this_arg = os->argv[os->ind];
       (*((svn_string_t **) apr_array_push (targets)))
- = svn_string_create (this_arg, pool);
+ = svn_string_create (subcommand->name, pool);
+
+ subcommand = svn_cl__get_canonical_command ("help");
+
+ err = (*subcommand->cmd_func) (&opt_state, targets, pool);
+ if (err)
+ svn_handle_error (err, stdout, 0);
+ return EXIT_FAILURE;
+ }
+ else
+ {
+ for (; os->ind < os->argc; os->ind++)
+ {
+ const char *this_arg = os->argv[os->ind];
+ (*((svn_string_t **) apr_array_push (targets)))
+ = svn_string_create (this_arg, pool);
+ }
     }

   /* Certain commands have an implied `.' as argument, if nothing else

This message: [ Message body ]
Next message: Karl Fogel: "Re: Check min and max num targets in client args patch."
Previous message: Karl Fogel: "Re: Patch to build db in the tree."
Next in thread: Karl Fogel: "Re: Check min and max num targets in client args patch."
Maybe reply: Karl Fogel: "Re: Check min and max num targets in client args patch."
Reply: B. W. Fitzpatrick: "Re: Check min and max num targets in client args patch."
Maybe reply: Karl Fogel: "Re: Check min and max num targets in client args patch."

Contemporary messages sorted: [ By Date ] [ By Thread ] [ By Subject ] [ By Author ] [ By messages with attachments ]