* [patch 1/2] ipset: remove the unneeded argvloop
2011-01-19 20:26 [patch 0/2] Ipset patches holger
@ 2011-01-19 20:26 ` holger
2011-01-19 20:26 ` [patch 2/2] ipset: remove " holger
2011-01-19 21:59 ` [patch 0/2] Ipset patches Jozsef Kadlecsik
2 siblings, 0 replies; 4+ messages in thread
From: holger @ 2011-01-19 20:26 UTC (permalink / raw)
To: Jozsef Kadlecsik; +Cc: netfilter-devel
[-- Attachment #1: ipset-enforce-command-order.diff --]
[-- Type: text/plain, Size: 3471 bytes --]
Remove the inner argv[] loop.
It is not a change functionality.
Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
Index: ipset/src/ipset.c
===================================================================
--- ipset.orig/src/ipset.c 2011-01-19 21:14:59.000000000 +0100
+++ ipset/src/ipset.c 2011-01-19 21:17:55.000000000 +0100
@@ -425,7 +425,6 @@
{
int ret = 0;
enum ipset_cmd cmd = IPSET_CMD_NONE;
- int i;
char *arg0 = NULL, *arg1 = NULL, *c;
const struct ipset_envopts *opt;
const struct ipset_commands *command;
@@ -476,61 +475,58 @@
/* Second: parse command */
for (command = ipset_commands;
- command->cmd && cmd == IPSET_CMD_NONE;
+ argc > 1 && command->cmd && cmd == IPSET_CMD_NONE;
command++) {
- for (i = 1; i < argc; ) {
- if (!ipset_match_cmd(argv[1], command->name)) {
- i++;
+ if (!ipset_match_cmd(argv[1], command->name))
continue;
- }
- if (restore_line != 0
- && (command->cmd == IPSET_CMD_RESTORE
- || command->cmd == IPSET_CMD_VERSION
- || command->cmd == IPSET_CMD_HELP))
- return exit_error(PARAMETER_PROBLEM,
+
+ if (restore_line != 0
+ && (command->cmd == IPSET_CMD_RESTORE
+ || command->cmd == IPSET_CMD_VERSION
+ || command->cmd == IPSET_CMD_HELP))
+ return exit_error(PARAMETER_PROBLEM,
"Command `%s' is invalid "
"in restore mode.",
command->name[0]);
- if (interactive
- && command->cmd == IPSET_CMD_RESTORE) {
- printf("Restore command ignored "
- "in interactive mode\n");
- return 0;
- }
+ if (interactive
+ && command->cmd == IPSET_CMD_RESTORE) {
+ printf("Restore command ignored "
+ "in interactive mode\n");
+ return 0;
+ }
- /* Shift off matched command arg */
- ipset_shift_argv(&argc, argv, i);
- cmd = command->cmd;
- switch (command->has_arg) {
- case IPSET_MANDATORY_ARG:
- case IPSET_MANDATORY_ARG2:
- if (i + 1 > argc)
+ /* Shift off matched command arg */
+ ipset_shift_argv(&argc, argv, 1);
+ cmd = command->cmd;
+ switch (command->has_arg) {
+ case IPSET_MANDATORY_ARG:
+ case IPSET_MANDATORY_ARG2:
+ if (argc < 2)
return exit_error(PARAMETER_PROBLEM,
- "Missing mandatory argument "
- "to command %s",
- command->name[0]);
+ "Missing mandatory argument "
+ "to command %s",
+ command->name[0]);
/* Fall through */
- case IPSET_OPTIONAL_ARG:
- arg0 = argv[i];
- if (i + 1 <= argc)
- /* Shift off first arg */
- ipset_shift_argv(&argc, argv, i);
- break;
- default:
- break;
- }
- if (command->has_arg == IPSET_MANDATORY_ARG2) {
- if (i + 1 > argc)
- return exit_error(PARAMETER_PROBLEM,
- "Missing second mandatory "
- "argument to command %s",
- command->name[0]);
- arg1 = argv[i];
- /* Shift off second arg */
- ipset_shift_argv(&argc, argv, i);
- }
+ case IPSET_OPTIONAL_ARG:
+ arg0 = argv[1];
+ if (argc >= 2)
+ /* Shift off first arg */
+ ipset_shift_argv(&argc, argv, 1);
+ break;
+ default:
break;
}
+ if (command->has_arg == IPSET_MANDATORY_ARG2) {
+ if (argc < 2)
+ return exit_error(PARAMETER_PROBLEM,
+ "Missing second mandatory "
+ "argument to command %s",
+ command->name[0]);
+ arg1 = argv[1];
+ /* Shift off second arg */
+ ipset_shift_argv(&argc, argv, 1);
+ }
+ break;
}
/* Third: catch interactive mode, handle help, version */
--
^ permalink raw reply [flat|nested] 4+ messages in thread* [patch 2/2] ipset: remove unneeded argvloop
2011-01-19 20:26 [patch 0/2] Ipset patches holger
2011-01-19 20:26 ` [patch 1/2] ipset: remove the unneeded argvloop holger
@ 2011-01-19 20:26 ` holger
2011-01-19 21:59 ` [patch 0/2] Ipset patches Jozsef Kadlecsik
2 siblings, 0 replies; 4+ messages in thread
From: holger @ 2011-01-19 20:26 UTC (permalink / raw)
To: Jozsef Kadlecsik; +Cc: netfilter-devel
[-- Attachment #1: ipset-enforce-command-argument-order.diff --]
[-- Type: text/plain, Size: 2821 bytes --]
It is not a change of functionality.
Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
Index: ipset/src/ipset.c
===================================================================
--- ipset.orig/src/ipset.c 2011-01-19 12:11:34.000000000 +0100
+++ ipset/src/ipset.c 2011-01-19 14:05:10.000000000 +0100
@@ -206,61 +206,56 @@
static int
call_parser(int *argc, char *argv[], const struct ipset_arg *args)
{
- int i = 1, ret = 0;
+ int ret = 0;
const struct ipset_arg *arg;
const char *optstr;
/* Currently CREATE and ADT may have got additional arguments */
if (!args)
goto done;
- for (arg = args; arg->opt; arg++) {
- for (i = 1; i < *argc; ) {
- D("argc: %u, i: %u: %s vs %s",
- *argc, i, argv[i], arg->name[0]);
- if (!(ipset_match_option(argv[i], arg->name))) {
- i++;
- continue;
- }
- optstr = argv[i];
- /* Shift off matched option */
- D("match %s", arg->name[0]);
- ipset_shift_argv(argc, argv, i);
- D("argc: %u, i: %u", *argc, i);
- switch (arg->has_arg) {
- case IPSET_MANDATORY_ARG:
- if (i + 1 > *argc)
- return exit_error(PARAMETER_PROBLEM,
- "Missing mandatory argument "
- "of option `%s'",
- arg->name[0]);
- /* Fall through */
- case IPSET_OPTIONAL_ARG:
- if (i + 1 <= *argc) {
- ret = ipset_call_parser(session,
- arg->parse,
- optstr, arg->opt,
- argv[i]);
- if (ret < 0)
- return ret;
- ipset_shift_argv(argc, argv, i);
- break;
- }
- /* Fall through */
- default:
+ for (arg = args; *argc > 1 && arg->opt; arg++) {
+ D("argc: %u: %s vs %s", *argc, argv[1], arg->name[0]);
+ if (!(ipset_match_option(argv[1], arg->name)))
+ continue;
+
+ optstr = argv[1];
+ /* Shift off matched option */
+ D("match %s", arg->name[0]);
+ ipset_shift_argv(argc, argv, 1);
+ D("argc: %u", *argc);
+ switch (arg->has_arg) {
+ case IPSET_MANDATORY_ARG:
+ if (*argc < 2)
+ return exit_error(PARAMETER_PROBLEM,
+ "Missing mandatory argument "
+ "of option `%s'",
+ arg->name[0]);
+ /* Fall through */
+ case IPSET_OPTIONAL_ARG:
+ if (*argc >= 2) {
ret = ipset_call_parser(session,
- arg->parse,
- optstr, arg->opt,
- optstr);
+ arg->parse,
+ optstr, arg->opt,
+ argv[1]);
if (ret < 0)
return ret;
+ ipset_shift_argv(argc, argv, 1);
+ break;
}
+ /* Fall through */
+ default:
+ ret = ipset_call_parser(session,
+ arg->parse,
+ optstr, arg->opt,
+ optstr);
+ if (ret < 0)
+ return ret;
}
}
done:
- if (i < *argc)
- return exit_error(PARAMETER_PROBLEM,
- "Unknown argument: `%s'",
- argv[i]);
+ if (*argc > 1)
+ return exit_error(PARAMETER_PROBLEM, "Unknown argument: `%s'",
+ argv[1]);
return ret;
}
--
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [patch 0/2] Ipset patches
2011-01-19 20:26 [patch 0/2] Ipset patches holger
2011-01-19 20:26 ` [patch 1/2] ipset: remove the unneeded argvloop holger
2011-01-19 20:26 ` [patch 2/2] ipset: remove " holger
@ 2011-01-19 21:59 ` Jozsef Kadlecsik
2 siblings, 0 replies; 4+ messages in thread
From: Jozsef Kadlecsik @ 2011-01-19 21:59 UTC (permalink / raw)
To: holger; +Cc: netfilter-devel
Hi Holger,
On Wed, 19 Jan 2011, holger@eitzenberger.org wrote:
> what follows are two small patches which remove unneeded argv[]
> loops when parsing the ipset commands. They are in particular no
> change in command line handling.
>
> The command line handling can still be improved, as e. g.
>
> $ ipset -o xml create foo hash:ip
> $ ipset -s add foo 192.168.1.1
>
> both work, but the options are just ignored. And they don't
> make much sense for either 'create' or 'add'.
>
> Also we should consider to make the argument handling a little
> more strict, as e. g.
>
> $ ipset -o xml list foo
> $ ipset list -o xml foo
> $ ipset list foo -o xml
>
> all work. By removing some of those possibilities it should even
> get simpler code wise.
Thank you indeed, after fixing the issues raised by Patrick and Eric, I'll
work on the userspace part and your patches.
Best regards,
Jozsef
-
E-mail : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
H-1525 Budapest 114, POB. 49, Hungary
^ permalink raw reply [flat|nested] 4+ messages in thread