[iptables PATCH 10/27] xshared: Consolidate argv construction routines

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Implementations were equal in {ip,ip6,x}tables-restore.c. The one in
iptables-xml.c differed slightly. For now, collect all features
together. Maybe it would make sense to migrate iptables-xml.c to using
add_param_to_argv() at some point and therefore extend the latter to
store whether a given parameter was quoted or not.

While being at it, a few improvements were done:

* free_argv() now also resets 'newargc' variable, so users don't have to
  do that anymore.
* Indenting level in add_param_to_argv() was reduced a bit.
* That long error message is put into a single line to aid in grepping
  for it.
* Explicit call to exit() after xtables_error() is removed since the
  latter does not return anyway.

Signed-off-by: Phil Sutter <phil@xxxxxx>
---
 iptables/ip6tables-restore.c | 110 ++-----------------------------
 iptables/iptables-restore.c  | 110 ++-----------------------------
 iptables/iptables-xml.c      |  64 ------------------
 iptables/xshared.c           | 123 +++++++++++++++++++++++++++++++++++
 iptables/xshared.h           |  13 ++++
 iptables/xtables-restore.c   | 115 ++++----------------------------
 6 files changed, 161 insertions(+), 374 deletions(-)

diff --git a/iptables/ip6tables-restore.c b/iptables/ip6tables-restore.c
index f2bd93d732d3c..51294f24ec904 100644
--- a/iptables/ip6tables-restore.c
+++ b/iptables/ip6tables-restore.c
@@ -79,99 +79,6 @@ static struct xtc_handle *create_handle(const char *tablename)
 	return handle;
 }
 
-/* global new argv and argc */
-static char *newargv[255];
-static int newargc;
-
-/* function adding one argument to newargv, updating newargc
- * returns true if argument added, false otherwise */
-static int add_argv(char *what) {
-	DEBUGP("add_argv: %s\n", what);
-	if (what && newargc + 1 < ARRAY_SIZE(newargv)) {
-		newargv[newargc] = strdup(what);
-		newargv[++newargc] = NULL;
-		return 1;
-	} else {
-		xtables_error(PARAMETER_PROBLEM,
-			"Parser cannot handle more arguments\n");
-		return 0;
-	}
-}
-
-static void free_argv(void) {
-	int i;
-
-	for (i = 0; i < newargc; i++)
-		free(newargv[i]);
-}
-
-static void add_param_to_argv(char *parsestart)
-{
-	int quote_open = 0, escaped = 0, param_len = 0;
-	char param_buffer[1024], *curchar;
-
-	/* After fighting with strtok enough, here's now
-	 * a 'real' parser. According to Rusty I'm now no
-	 * longer a real hacker, but I can live with that */
-
-	for (curchar = parsestart; *curchar; curchar++) {
-		if (quote_open) {
-			if (escaped) {
-				param_buffer[param_len++] = *curchar;
-				escaped = 0;
-				continue;
-			} else if (*curchar == '\\') {
-				escaped = 1;
-				continue;
-			} else if (*curchar == '"') {
-				quote_open = 0;
-				*curchar = ' ';
-			} else {
-				param_buffer[param_len++] = *curchar;
-				continue;
-			}
-		} else {
-			if (*curchar == '"') {
-				quote_open = 1;
-				continue;
-			}
-		}
-
-		if (*curchar == ' '
-		    || *curchar == '\t'
-		    || * curchar == '\n') {
-			if (!param_len) {
-				/* two spaces? */
-				continue;
-			}
-
-			param_buffer[param_len] = '\0';
-
-			/* check if table name specified */
-			if ((param_buffer[0] == '-' &&
-			     param_buffer[1] != '-' &&
-			     strchr(param_buffer, 't')) ||
-			    (!strncmp(param_buffer, "--t", 3) &&
-			     !strncmp(param_buffer, "--table", strlen(param_buffer)))) {
-				xtables_error(PARAMETER_PROBLEM,
-				"The -t option (seen in line %u) cannot be "
-				"used in ip6tables-restore.\n", line);
-				exit(1);
-			}
-
-			add_argv(param_buffer);
-			param_len = 0;
-		} else {
-			/* regular character, copy to buffer */
-			param_buffer[param_len++] = *curchar;
-
-			if (param_len >= sizeof(param_buffer))
-				xtables_error(PARAMETER_PROBLEM,
-				   "Parameter too long!");
-		}
-	}
-}
-
 int ip6tables_restore_main(int argc, char *argv[])
 {
 	struct xtc_handle *handle = NULL;
@@ -414,9 +321,6 @@ int ip6tables_restore_main(int argc, char *argv[])
 			char *bcnt = NULL;
 			char *parsestart;
 
-			/* reset the newargv */
-			newargc = 0;
-
 			if (buffer[0] == '[') {
 				/* we have counters in our input */
 				ptr = strchr(buffer, ']');
@@ -444,17 +348,17 @@ int ip6tables_restore_main(int argc, char *argv[])
 				parsestart = buffer;
 			}
 
-			add_argv(argv[0]);
-			add_argv("-t");
-			add_argv(curtable);
+			add_argv(argv[0], 0);
+			add_argv("-t", 0);
+			add_argv(curtable, 0);
 
 			if (counters && pcnt && bcnt) {
-				add_argv("--set-counters");
-				add_argv((char *) pcnt);
-				add_argv((char *) bcnt);
+				add_argv("--set-counters", 0);
+				add_argv((char *) pcnt, 0);
+				add_argv((char *) bcnt, 0);
 			}
 
-			add_param_to_argv(parsestart);
+			add_param_to_argv(parsestart, line);
 
 			DEBUGP("calling do_command6(%u, argv, &%s, handle):\n",
 				newargc, curtable);
diff --git a/iptables/iptables-restore.c b/iptables/iptables-restore.c
index a1ae0311f508b..f596b46c7dce2 100644
--- a/iptables/iptables-restore.c
+++ b/iptables/iptables-restore.c
@@ -76,99 +76,6 @@ static struct xtc_handle *create_handle(const char *tablename)
 	return handle;
 }
 
-/* global new argv and argc */
-static char *newargv[255];
-static int newargc;
-
-/* function adding one argument to newargv, updating newargc 
- * returns true if argument added, false otherwise */
-static int add_argv(char *what) {
-	DEBUGP("add_argv: %s\n", what);
-	if (what && newargc + 1 < ARRAY_SIZE(newargv)) {
-		newargv[newargc] = strdup(what);
-		newargv[++newargc] = NULL;
-		return 1;
-	} else {
-		xtables_error(PARAMETER_PROBLEM,
-			"Parser cannot handle more arguments\n");
-		return 0;
-	}
-}
-
-static void free_argv(void) {
-	int i;
-
-	for (i = 0; i < newargc; i++)
-		free(newargv[i]);
-}
-
-static void add_param_to_argv(char *parsestart)
-{
-	int quote_open = 0, escaped = 0, param_len = 0;
-	char param_buffer[1024], *curchar;
-
-	/* After fighting with strtok enough, here's now
-	 * a 'real' parser. According to Rusty I'm now no
-	 * longer a real hacker, but I can live with that */
-
-	for (curchar = parsestart; *curchar; curchar++) {
-		if (quote_open) {
-			if (escaped) {
-				param_buffer[param_len++] = *curchar;
-				escaped = 0;
-				continue;
-			} else if (*curchar == '\\') {
-				escaped = 1;
-				continue;
-			} else if (*curchar == '"') {
-				quote_open = 0;
-				*curchar = ' ';
-			} else {
-				param_buffer[param_len++] = *curchar;
-				continue;
-			}
-		} else {
-			if (*curchar == '"') {
-				quote_open = 1;
-				continue;
-			}
-		}
-
-		if (*curchar == ' '
-		    || *curchar == '\t'
-		    || * curchar == '\n') {
-			if (!param_len) {
-				/* two spaces? */
-				continue;
-			}
-
-			param_buffer[param_len] = '\0';
-
-			/* check if table name specified */
-			if ((param_buffer[0] == '-' &&
-			     param_buffer[1] != '-' &&
-			     strchr(param_buffer, 't')) ||
-			    (!strncmp(param_buffer, "--t", 3) &&
-			     !strncmp(param_buffer, "--table", strlen(param_buffer)))) {
-				xtables_error(PARAMETER_PROBLEM,
-				"The -t option (seen in line %u) cannot be "
-				"used in iptables-restore.\n", line);
-				exit(1);
-			}
-
-			add_argv(param_buffer);
-			param_len = 0;
-		} else {
-			/* regular character, copy to buffer */
-			param_buffer[param_len++] = *curchar;
-
-			if (param_len >= sizeof(param_buffer))
-				xtables_error(PARAMETER_PROBLEM,
-				   "Parameter too long!");
-		}
-	}
-}
-
 int
 iptables_restore_main(int argc, char *argv[])
 {
@@ -412,9 +319,6 @@ iptables_restore_main(int argc, char *argv[])
 			char *bcnt = NULL;
 			char *parsestart;
 
-			/* reset the newargv */
-			newargc = 0;
-
 			if (buffer[0] == '[') {
 				/* we have counters in our input */
 				ptr = strchr(buffer, ']');
@@ -442,17 +346,17 @@ iptables_restore_main(int argc, char *argv[])
 				parsestart = buffer;
 			}
 
-			add_argv(argv[0]);
-			add_argv("-t");
-			add_argv(curtable);
+			add_argv(argv[0], 0);
+			add_argv("-t", 0);
+			add_argv(curtable, 0);
 
 			if (counters && pcnt && bcnt) {
-				add_argv("--set-counters");
-				add_argv((char *) pcnt);
-				add_argv((char *) bcnt);
+				add_argv("--set-counters", 0);
+				add_argv((char *) pcnt, 0);
+				add_argv((char *) bcnt, 0);
 			}
 
-			add_param_to_argv(parsestart);
+			add_param_to_argv(parsestart, line);
 
 			DEBUGP("calling do_command4(%u, argv, &%s, handle):\n",
 				newargc, curtable);
diff --git a/iptables/iptables-xml.c b/iptables/iptables-xml.c
index 8ba45d55c079c..788a67c608ec6 100644
--- a/iptables/iptables-xml.c
+++ b/iptables/iptables-xml.c
@@ -50,16 +50,6 @@ print_usage(const char *name, const char *version)
 	exit(1);
 }
 
-/* global new argv and argc */
-static char *newargv[255];
-static unsigned int newargc;
-
-static char *oldargv[255];
-static unsigned int oldargc;
-
-/* arg meta data, were they quoted, frinstance */
-static int newargvattr[255];
-
 #define XT_CHAIN_MAXNAMELEN XT_TABLE_MAXNAMELEN
 static char closeActionTag[XT_TABLE_MAXNAMELEN + 1];
 static char closeRuleTag[XT_TABLE_MAXNAMELEN + 1];
@@ -77,57 +67,6 @@ struct chain {
 static struct chain chains[maxChains];
 static int nextChain;
 
-/* funCtion adding one argument to newargv, updating newargc 
- * returns true if argument added, false otherwise */
-static int
-add_argv(char *what, int quoted)
-{
-	DEBUGP("add_argv: %d %s\n", newargc, what);
-	if (what && newargc + 1 < ARRAY_SIZE(newargv)) {
-		newargv[newargc] = strdup(what);
-		newargvattr[newargc] = quoted;
-		newargc++;
-		return 1;
-	} else
-		return 0;
-}
-
-static void
-free_argv(void)
-{
-	unsigned int i;
-
-	for (i = 0; i < newargc; i++) {
-		free(newargv[i]);
-		newargv[i] = NULL;
-	}
-	newargc = 0;
-
-	for (i = 0; i < oldargc; i++) {
-		free(oldargv[i]);
-		oldargv[i] = NULL;
-	}
-	oldargc = 0;
-}
-
-/* Save parsed rule for comparison with next rule to perform action aggregation
- * on duplicate conditions.
- */
-static void
-save_argv(void)
-{
-	unsigned int i;
-
-	for (i = 0; i < oldargc; i++)
-		free(oldargv[i]);
-	oldargc = newargc;
-	newargc = 0;
-	for (i = 0; i < oldargc; i++) {
-		oldargv[i] = newargv[i];
-		newargv[i] = NULL;
-	}
-}
-
 /* like puts but with xml encoding */
 static void
 xmlEncode(char *text)
@@ -720,9 +659,6 @@ iptables_xml_main(int argc, char *argv[])
 			int quote_open, quoted;
 			char param_buffer[1024];
 
-			/* reset the newargv */
-			newargc = 0;
-
 			if (buffer[0] == '[') {
 				/* we have counters in our input */
 				ptr = strchr(buffer, ']');
diff --git a/iptables/xshared.c b/iptables/xshared.c
index ec5c49556b38d..436eadad32115 100644
--- a/iptables/xshared.c
+++ b/iptables/xshared.c
@@ -379,3 +379,126 @@ inline bool xs_has_arg(int argc, char *argv[])
 	       argv[optind][0] != '-' &&
 	       argv[optind][0] != '!';
 }
+
+/* global new argv and argc */
+char *newargv[255];
+int newargc = 0;
+
+/* saved newargv and newargc from save_argv() */
+char *oldargv[255];
+int oldargc = 0;
+
+/* arg meta data, were they quoted, frinstance */
+int newargvattr[255];
+
+/* function adding one argument to newargv, updating newargc 
+ * returns true if argument added, false otherwise */
+int add_argv(const char *what, int quoted)
+{
+	DEBUGP("add_argv: %s\n", what);
+	if (what && newargc + 1 < ARRAY_SIZE(newargv)) {
+		newargv[newargc] = strdup(what);
+		newargvattr[newargc] = quoted;
+		newargv[++newargc] = NULL;
+		return 1;
+	} else {
+		xtables_error(PARAMETER_PROBLEM,
+			      "Parser cannot handle more arguments\n");
+	}
+}
+
+void free_argv(void)
+{
+	while (newargc)
+		free(newargv[--newargc]);
+	while (oldargc)
+		free(oldargv[--oldargc]);
+}
+
+/* Save parsed rule for comparison with next rule to perform action aggregation
+ * on duplicate conditions.
+ */
+void save_argv(void)
+{
+	unsigned int i;
+
+	while (oldargc)
+		free(oldargv[--oldargc]);
+
+	oldargc = newargc;
+	newargc = 0;
+	for (i = 0; i < oldargc; i++) {
+		oldargv[i] = newargv[i];
+	}
+}
+
+void add_param_to_argv(char *parsestart, int line)
+{
+	int quote_open = 0, escaped = 0, param_len = 0;
+	char param_buffer[1024], *curchar;
+
+	/* After fighting with strtok enough, here's now
+	 * a 'real' parser. According to Rusty I'm now no
+	 * longer a real hacker, but I can live with that */
+
+	for (curchar = parsestart; *curchar; curchar++) {
+		if (quote_open) {
+			if (escaped) {
+				param_buffer[param_len++] = *curchar;
+				escaped = 0;
+				continue;
+			} else if (*curchar == '\\') {
+				escaped = 1;
+				continue;
+			} else if (*curchar == '"') {
+				quote_open = 0;
+				*curchar = '"';
+			} else {
+				param_buffer[param_len++] = *curchar;
+				continue;
+			}
+		} else {
+			if (*curchar == '"') {
+				quote_open = 1;
+				continue;
+			}
+		}
+
+		switch (*curchar) {
+		case '"':
+			break;
+		case ' ':
+		case '\t':
+		case '\n':
+			if (!param_len) {
+				/* two spaces? */
+				continue;
+			}
+			break;
+		default:
+			/* regular character, copy to buffer */
+			param_buffer[param_len++] = *curchar;
+
+			if (param_len >= sizeof(param_buffer))
+				xtables_error(PARAMETER_PROBLEM,
+					      "Parameter too long!");
+			continue;
+		}
+
+		param_buffer[param_len] = '\0';
+
+		/* check if table name specified */
+		if ((param_buffer[0] == '-' &&
+		     param_buffer[1] != '-' &&
+		     strchr(param_buffer, 't')) ||
+		    (!strncmp(param_buffer, "--t", 3) &&
+		     !strncmp(param_buffer, "--table", strlen(param_buffer)))) {
+			xtables_error(PARAMETER_PROBLEM,
+				      "The -t option (seen in line %u) cannot be used in %s.\n",
+				      line, xt_params->program_name);
+		}
+
+		add_argv(param_buffer, 0);
+		param_len = 0;
+	}
+}
diff --git a/iptables/xshared.h b/iptables/xshared.h
index 55249341a19ba..801d0f7564dc4 100644
--- a/iptables/xshared.h
+++ b/iptables/xshared.h
@@ -155,4 +155,17 @@ bool xs_has_arg(int argc, char *argv[]);
 
 extern const struct xtables_afinfo *afinfo;
 
+extern char *newargv[];
+extern int newargc;
+
+extern char *oldargv[];
+extern int oldargc;
+
+extern int newargvattr[];
+
+int add_argv(const char *what, int quoted);
+void free_argv(void);
+void save_argv(void);
+void add_param_to_argv(char *parsestart, int line);
+
 #endif /* IPTABLES_XSHARED_H */
diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index 018d3fd3c80b0..60e07f78b38df 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -56,99 +56,6 @@ static void print_usage(const char *name, const char *version)
 			"	   [ --ipv6 ]\n", name);
 }
 
-/* global new argv and argc */
-static char *newargv[255];
-static int newargc;
-
-/* function adding one argument to newargv, updating newargc 
- * returns true if argument added, false otherwise */
-static int add_argv(const char *what) {
-	DEBUGP("add_argv: %s\n", what);
-	if (what && newargc + 1 < ARRAY_SIZE(newargv)) {
-		newargv[newargc] = strdup(what);
-		newargv[++newargc] = NULL;
-		return 1;
-	} else {
-		xtables_error(PARAMETER_PROBLEM,
-			"Parser cannot handle more arguments\n");
-		return 0;
-	}
-}
-
-static void free_argv(void) {
-	int i;
-
-	for (i = 0; i < newargc; i++)
-		free(newargv[i]);
-}
-
-static void add_param_to_argv(char *parsestart)
-{
-	int quote_open = 0, escaped = 0, param_len = 0;
-	char param_buffer[1024], *curchar;
-
-	/* After fighting with strtok enough, here's now
-	 * a 'real' parser. According to Rusty I'm now no
-	 * longer a real hacker, but I can live with that */
-
-	for (curchar = parsestart; *curchar; curchar++) {
-		if (quote_open) {
-			if (escaped) {
-				param_buffer[param_len++] = *curchar;
-				escaped = 0;
-				continue;
-			} else if (*curchar == '\\') {
-				escaped = 1;
-				continue;
-			} else if (*curchar == '"') {
-				quote_open = 0;
-				*curchar = ' ';
-			} else {
-				param_buffer[param_len++] = *curchar;
-				continue;
-			}
-		} else {
-			if (*curchar == '"') {
-				quote_open = 1;
-				continue;
-			}
-		}
-
-		if (*curchar == ' '
-		    || *curchar == '\t'
-		    || * curchar == '\n') {
-			if (!param_len) {
-				/* two spaces? */
-				continue;
-			}
-
-			param_buffer[param_len] = '\0';
-
-			/* check if table name specified */
-			if ((param_buffer[0] == '-' &&
-			     param_buffer[1] != '-' &&
-			     strchr(param_buffer, 't')) ||
-			    (!strncmp(param_buffer, "--t", 3) &&
-			     !strncmp(param_buffer, "--table", strlen(param_buffer)))) {
-				xtables_error(PARAMETER_PROBLEM,
-				"The -t option (seen in line %u) cannot be "
-				"used in xtables-restore.\n", line);
-				exit(1);
-			}
-
-			add_argv(param_buffer);
-			param_len = 0;
-		} else {
-			/* regular character, copy to buffer */
-			param_buffer[param_len++] = *curchar;
-
-			if (param_len >= sizeof(param_buffer))
-				xtables_error(PARAMETER_PROBLEM,
-				   "Parameter too long!");
-		}
-	}
-}
-
 static struct nftnl_chain_list *get_chain_list(struct nft_handle *h)
 {
 	struct nftnl_chain_list *chain_list;
@@ -385,17 +292,17 @@ void xtables_restore_parse(struct nft_handle *h,
 				parsestart = buffer;
 			}
 
-			add_argv(argv[0]);
-			add_argv("-t");
-			add_argv(curtable);
+			add_argv(argv[0], 0);
+			add_argv("-t", 0);
+			add_argv(curtable, 0);
 
 			if (counters && pcnt && bcnt) {
-				add_argv("--set-counters");
-				add_argv((char *) pcnt);
-				add_argv((char *) bcnt);
+				add_argv("--set-counters", 0);
+				add_argv((char *) pcnt, 0);
+				add_argv((char *) bcnt, 0);
 			}
 
-			add_param_to_argv(parsestart);
+			add_param_to_argv(parsestart, line);
 
 			DEBUGP("calling do_command4(%u, argv, &%s, handle):\n",
 				newargc, curtable);
@@ -656,10 +563,10 @@ int xtables_eb_restore_main(int argc, char *argv[])
 		}
 
 		newargc = 0;
-		add_argv("ebtables");
-		add_argv("-t");
-		add_argv(table);
-		add_param_to_argv(buffer);
+		add_argv("ebtables", 0);
+		add_argv("-t", 0);
+		add_argv(table, 0);
+		add_param_to_argv(buffer, line);
 
 		DEBUGP("calling do_commandeb(%u, argv, &%s, handle):\n",
 			newargc, table);
-- 
2.18.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux