xshared: Consolidate argv construction routines

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@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>

diff --git a/iptables/ip6tables-restore.c b/iptables/ip6tables-restore.c
index f2bd93d732d3c1dbfc994f7d7b9e65176bdf5d60..51294f24ec90417743149abf01896e2991053c6c 100644 (file)
--- 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 a1ae0311f508bc970c32cd709c75ab7ccc2bc766..f596b46c7dce2ff4c76c1584270dc6ae3815dbf7 100644 (file)
--- 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 8ba45d55c079c442d2dff16d5418c9f88bba4e90..788a67c608ec67111d407423cbf276a394f60414 100644 (file)
--- 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 ec5c49556b38d21d3765dbf768746a8ed4079da9..492e0087379562d1f2441201b492953574e059db 100644 (file)
--- 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 55249341a19ba81151a59679eeba563e7daacf8d..801d0f7564dc48137d8d05b7d344fb4a6fd141b2 100644 (file)
--- 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 018d3fd3c80b097e80ba8bf2ef6bc11fc925e8c5..60e07f78b38dfbf03e3be927d58f454708f88d19 100644 (file)
--- 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);