All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Holger Eitzenberger" <holger@eitzenberger.org>
To: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Cc: netfilter-devel@vger.kernel.org
Subject: [patch 5/6] ipset: improve command argument parsing
Date: Mon, 24 Jan 2011 22:36:36 +0100	[thread overview]
Message-ID: <20110124214014.604070478@jonathan.eitzenberger.org> (raw)
In-Reply-To: 20110124213631.195499507@jonathan.eitzenberger.org

[-- Attachment #1: ipset-improve-cmd-argument-parsing.diff --]
[-- Type: text/plain, Size: 2669 bytes --]

The number of comparisons for a matching a command name can be
made smaller by just checking on argv[1].

As an example consider the following 'create' arguments 'hashsize',
'family' and 'timeout'.  When having the command
 
 create foo hash:ip timeout 60 family inet hashsize 64

it compares without this patch:

 strcmp("timeout", "hashsize")
 strcmp("64", "hashsize")
 strcmp("family", "hashsize")
 strcmp("inet", "hashsize")
 strcmp("hashsize", "hashsize")

It is worse in practice, as 'create' has more arguments than this.

Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>

Index: ipset/src/ipset.c
===================================================================
--- ipset.orig/src/ipset.c
+++ ipset/src/ipset.c
@@ -206,38 +206,35 @@ restore(char *argv0)
 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;
 	
 	/* 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++;
+	if (!args && *argc > 1)
+		goto err_unknown;
+	while (*argc > 1) {
+		for (arg = args; arg->opt; arg++) {
+			D("argc: %u, %s vs %s", *argc, argv[1], arg->name[0]);
+			if (!(ipset_match_option(argv[1], arg->name)))
 				continue;
-			}
+
 			/* Shift off matched option */
 			D("match %s", arg->name[0]);
-			ipset_shift_argv(argc, argv, i);
-			D("argc: %u, i: %u", *argc, i);
+			ipset_shift_argv(argc, argv, 1);
 			switch (arg->has_arg) {
 			case IPSET_MANDATORY_ARG:
-				if (i + 1 > *argc)
+				if (*argc < 2)
 					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, argv[i]);
+				if (*argc >= 2) {
+					ret = ipset_call_parser(session, arg, argv[1]);
 					if (ret < 0)
 						return ret;
-					ipset_shift_argv(argc, argv, i);
+					ipset_shift_argv(argc, argv, 1);
 					break;
 				}
 				/* Fall through */
@@ -246,14 +243,18 @@ call_parser(int *argc, char *argv[], con
 				if (ret < 0)
 					return ret;
 			}
+
+			break;
 		}
+
+		if (!arg->opt)
+			goto err_unknown;
 	}
-done:
-	if (i < *argc)
-		return exit_error(PARAMETER_PROBLEM,
-				  "Unknown argument: `%s'",
-				  argv[i]);
+
 	return ret;
+
+err_unknown:
+	return exit_error(PARAMETER_PROBLEM, "Unknown argument: `%s'", argv[1]);
 }
 
 static enum ipset_adt


  parent reply	other threads:[~2011-01-24 21:40 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-24 21:36 [patch 0/6] Ipset patches Holger Eitzenberger
2011-01-24 21:36 ` [patch 1/6] ipset: turn Set name[] into a const pointer Holger Eitzenberger
2011-01-25 20:26   ` Jozsef Kadlecsik
2011-01-24 21:36 ` [patch 2/6] ipset: fix the Netlink sequence number Holger Eitzenberger
2011-01-25 20:26   ` Jozsef Kadlecsik
2011-01-24 21:36 ` [patch 3/6] ipset: pass ipset_arg argument pointer Holger Eitzenberger
2011-01-25 20:42   ` Jozsef Kadlecsik
2011-01-27 10:06     ` Jozsef Kadlecsik
2011-02-01 20:10       ` Jozsef Kadlecsik
2011-01-24 21:36 ` [patch 4/6] ipset: avoid the unnecessary argv[] loop Holger Eitzenberger
2011-01-24 21:36 ` Holger Eitzenberger [this message]
2011-01-24 21:36 ` [patch 6/6] ipset: fix spelling error Holger Eitzenberger
2011-01-25 20:43   ` Jozsef Kadlecsik
2011-01-26 17:57   ` Ferenc Wagner
2011-01-26 22:53     ` Jozsef Kadlecsik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110124214014.604070478@jonathan.eitzenberger.org \
    --to=holger@eitzenberger.org \
    --cc=kadlec@blackhole.kfki.hu \
    --cc=netfilter-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.