summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPablo Neira Ayuso <pablo@netfilter.org>2012-07-23 12:27:16 +0200
committerPablo Neira Ayuso <pablo@netfilter.org>2012-07-25 13:04:52 +0200
commit44191bdbd71e685fba9eab864b9df25e63905220 (patch)
treeaca9c181f8c0643374653870e6742db51dce2a4b
parentf4a6c20c39c97214e22625764bfa80ef8e1e3147 (diff)
iptables-restore: move code to add_param_to_argv, cleanup (fix gcc-4.7)
This patch seems to be a mere cleanup that moves the parameter parsing code to add_param_to_argv. But, in reality, it also fixes iptables when compiled with gcc-4.7. Moving param_buffer declaration out of the loop seems to resolve the issue. gcc-4.7 seems to be generating bad code regarding param_buffer. @@ -380,9 +380,9 @@ quote_open = 0; escaped = 0; param_len = 0; + char param_buffer[1024]; for (curchar = parsestart; *curchar; curchar++) { - char param_buffer[1024]; if (quote_open) { if (escaped) { But I have hard time to apply this patch in such a way. Instead, I came up with the idea of this cleanup, which does not harm after all (and fixes the issue for us). Someone in: https://bugzilla.redhat.com/show_bug.cgi?id=82579 put some light on this: "Yes, I ran into this too. The issue is that the gcc optimizer is optimizing out the code that collects quoted strings in iptables-restore.c at line 396. If inside a quotemark and it hasn't seen another one yet, it executes param_buffer[param_len++] = *curchar; continue; At -O1 or higher, the write to param_buffer[] never happens. It just increments param_len and continues. Moving the definition of char param_buffer[1024]; outside the loop fixes it. Why, I'm not sure. Defining the param_buffer[] inside the loop should simply restrict its scope to inside the loop." Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
-rw-r--r--iptables/ip6tables-restore.c133
-rw-r--r--iptables/iptables-restore.c133
2 files changed, 130 insertions, 136 deletions
diff --git a/iptables/ip6tables-restore.c b/iptables/ip6tables-restore.c
index 3894d68d..9a03dff4 100644
--- a/iptables/ip6tables-restore.c
+++ b/iptables/ip6tables-restore.c
@@ -114,6 +114,70 @@ static void free_argv(void) {
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 (!strncmp(param_buffer, "-t", 2)
+ || !strncmp(param_buffer, "--table", 8)) {
+ xtables_error(PARAMETER_PROBLEM,
+ "Line %u seems to have a "
+ "-t table option.\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;
@@ -325,11 +389,6 @@ int ip6tables_restore_main(int argc, char *argv[])
char *bcnt = NULL;
char *parsestart;
- /* the parser */
- char *curchar;
- int quote_open, escaped;
- size_t param_len;
-
/* reset the newargv */
newargc = 0;
@@ -370,69 +429,7 @@ int ip6tables_restore_main(int argc, char *argv[])
add_argv((char *) bcnt);
}
- /* 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 */
-
- quote_open = 0;
- escaped = 0;
- param_len = 0;
-
- for (curchar = parsestart; *curchar; curchar++) {
- char param_buffer[1024];
-
- 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 (!strncmp(param_buffer, "-t", 2)
- || !strncmp(param_buffer, "--table", 8)) {
- xtables_error(PARAMETER_PROBLEM,
- "Line %u seems to have a "
- "-t table option.\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!");
- }
- }
+ add_param_to_argv(parsestart);
DEBUGP("calling do_command6(%u, argv, &%s, handle):\n",
newargc, curtable);
diff --git a/iptables/iptables-restore.c b/iptables/iptables-restore.c
index 034f9606..c974cb37 100644
--- a/iptables/iptables-restore.c
+++ b/iptables/iptables-restore.c
@@ -113,6 +113,70 @@ static void free_argv(void) {
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 (!strncmp(param_buffer, "-t", 2)
+ || !strncmp(param_buffer, "--table", 8)) {
+ xtables_error(PARAMETER_PROBLEM,
+ "Line %u seems to have a "
+ "-t table option.\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[])
{
@@ -325,11 +389,6 @@ iptables_restore_main(int argc, char *argv[])
char *bcnt = NULL;
char *parsestart;
- /* the parser */
- char *curchar;
- int quote_open, escaped;
- size_t param_len;
-
/* reset the newargv */
newargc = 0;
@@ -370,69 +429,7 @@ iptables_restore_main(int argc, char *argv[])
add_argv((char *) bcnt);
}
- /* 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 */
-
- quote_open = 0;
- escaped = 0;
- param_len = 0;
-
- for (curchar = parsestart; *curchar; curchar++) {
- char param_buffer[1024];
-
- 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 (!strncmp(param_buffer, "-t", 2)
- || !strncmp(param_buffer, "--table", 8)) {
- xtables_error(PARAMETER_PROBLEM,
- "Line %u seems to have a "
- "-t table option.\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!");
- }
- }
+ add_param_to_argv(parsestart);
DEBUGP("calling do_command4(%u, argv, &%s, handle):\n",
newargc, curtable);