[iptables PATCH v2 20/24] xtables-restore: Introduce line parsing function

[Phil Sutter <phil@nwl.cc>]

Move the loop code parsing a distinct line of input into a dedicated
function as a preparation for changing input sources. Since loop code
either calls continue or exit() directly, there is no need for a return
code to indicate failure.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Despite the diff's size, this is actually a small change: Pass
--ignore-space-change to git to see how few changes there actually are
besides the necessary indenting level change.
---
 iptables/xtables-restore.c | 354 +++++++++++++++++++------------------
 1 file changed, 180 insertions(+), 174 deletions(-)

diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index 3bd8a8925c8bc..1a3739b3350c8 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -84,199 +84,205 @@ static const struct xtc_ops xtc_ops = {
 	.strerror	= nft_strerror,
 };
 
-void xtables_restore_parse(struct nft_handle *h,
-			   struct nft_xt_restore_parse *p,
-			   struct nft_xt_restore_cb *cb)
+static void xtables_restore_parse_line(struct nft_handle *h,
+				       struct nft_xt_restore_parse *p,
+				       struct nft_xt_restore_cb *cb,
+				       char *buffer)
 {
-	char buffer[10240];
 	const struct xtc_ops *ops = &xtc_ops;
+	int ret = 0;
+
+	if (buffer[0] == '\n')
+		return;
+	else if (buffer[0] == '#') {
+		if (verbose)
+			fputs(buffer, stdout);
+		return;
+	} else if ((strcmp(buffer, "COMMIT\n") == 0) && (p->in_table)) {
+		if (!p->testing) {
+			/* Commit per table, although we support
+			 * global commit at once, stick by now to
+			 * the existing behaviour.
+			 */
+			DEBUGP("Calling commit\n");
+			if (cb->commit)
+				ret = cb->commit(h);
+		} else {
+			DEBUGP("Not calling commit, testing\n");
+			if (cb->abort)
+				ret = cb->abort(h);
+		}
+		p->in_table = 0;
+
+	} else if ((buffer[0] == '*') && (!p->in_table || !p->commit)) {
+		/* New table */
+		char *table;
+
+		table = strtok(buffer+1, " \t\n");
+		DEBUGP("line %u, table '%s'\n", line, table);
+		if (!table)
+			xtables_error(PARAMETER_PROBLEM,
+				"%s: line %u table name invalid\n",
+				xt_params->program_name, line);
+
+		p->curtable = nft_table_builtin_find(h, table);
+		if (!p->curtable)
+			xtables_error(PARAMETER_PROBLEM,
+				"%s: line %u table name '%s' invalid\n",
+				xt_params->program_name, line, table);
+
+		if (p->tablename && (strcmp(p->tablename, table) != 0))
+			return;
+
+		if (h->noflush == 0) {
+			DEBUGP("Cleaning all chains of table '%s'\n",
+				table);
+			if (cb->table_flush)
+				cb->table_flush(h, table);
+		}
 
-	line = 0;
-
-	if (!h->noflush)
-		nft_fake_cache(h);
-	else
-		nft_build_cache(h);
+		ret = 1;
+		p->in_table = 1;
+
+		if (cb->table_new)
+			cb->table_new(h, table);
+
+	} else if ((buffer[0] == ':') && (p->in_table)) {
+		/* New chain. */
+		char *policy, *chain = NULL;
+		struct xt_counters count = {};
+
+		chain = strtok(buffer+1, " \t\n");
+		DEBUGP("line %u, chain '%s'\n", line, chain);
+		if (!chain)
+			xtables_error(PARAMETER_PROBLEM,
+				   "%s: line %u chain name invalid\n",
+				   xt_params->program_name, line);
+
+		if (strlen(chain) >= XT_EXTENSION_MAXNAMELEN)
+			xtables_error(PARAMETER_PROBLEM,
+				   "Invalid chain name `%s' "
+				   "(%u chars max)",
+				   chain, XT_EXTENSION_MAXNAMELEN - 1);
+
+		policy = strtok(NULL, " \t\n");
+		DEBUGP("line %u, policy '%s'\n", line, policy);
+		if (!policy)
+			xtables_error(PARAMETER_PROBLEM,
+				   "%s: line %u policy invalid\n",
+				   xt_params->program_name, line);
+
+		if (nft_chain_builtin_find(p->curtable, chain)) {
+			if (counters) {
+				char *ctrs;
+				ctrs = strtok(NULL, " \t\n");
+
+				if (!ctrs || !parse_counters(ctrs, &count))
+					xtables_error(PARAMETER_PROBLEM,
+						   "invalid policy counters "
+						   "for chain '%s'\n", chain);
 
-	/* Grab standard input. */
-	while (fgets(buffer, sizeof(buffer), p->in)) {
-		int ret = 0;
-
-		line++;
-		h->error.lineno = line;
-
-		if (buffer[0] == '\n')
-			continue;
-		else if (buffer[0] == '#') {
-			if (verbose)
-				fputs(buffer, stdout);
-			continue;
-		} else if ((strcmp(buffer, "COMMIT\n") == 0) && (p->in_table)) {
-			if (!p->testing) {
-				/* Commit per table, although we support
-				 * global commit at once, stick by now to
-				 * the existing behaviour.
-				 */
-				DEBUGP("Calling commit\n");
-				if (cb->commit)
-					ret = cb->commit(h);
-			} else {
-				DEBUGP("Not calling commit, testing\n");
-				if (cb->abort)
-					ret = cb->abort(h);
 			}
-			p->in_table = 0;
-
-		} else if ((buffer[0] == '*') && (!p->in_table || !p->commit)) {
-			/* New table */
-			char *table;
-
-			table = strtok(buffer+1, " \t\n");
-			DEBUGP("line %u, table '%s'\n", line, table);
-			if (!table)
-				xtables_error(PARAMETER_PROBLEM,
-					"%s: line %u table name invalid\n",
-					xt_params->program_name, line);
-
-			p->curtable = nft_table_builtin_find(h, table);
-			if (!p->curtable)
-				xtables_error(PARAMETER_PROBLEM,
-					"%s: line %u table name '%s' invalid\n",
-					xt_params->program_name, line, table);
-
-			if (p->tablename && (strcmp(p->tablename, table) != 0))
-				continue;
-
-			if (h->noflush == 0) {
-				DEBUGP("Cleaning all chains of table '%s'\n",
-					table);
-				if (cb->table_flush)
-					cb->table_flush(h, table);
-			}
-
-			ret = 1;
-			p->in_table = 1;
-
-			if (cb->table_new)
-				cb->table_new(h, table);
-
-		} else if ((buffer[0] == ':') && (p->in_table)) {
-			/* New chain. */
-			char *policy, *chain = NULL;
-			struct xt_counters count = {};
-
-			chain = strtok(buffer+1, " \t\n");
-			DEBUGP("line %u, chain '%s'\n", line, chain);
-			if (!chain)
-				xtables_error(PARAMETER_PROBLEM,
-					   "%s: line %u chain name invalid\n",
-					   xt_params->program_name, line);
-
-			if (strlen(chain) >= XT_EXTENSION_MAXNAMELEN)
-				xtables_error(PARAMETER_PROBLEM,
-					   "Invalid chain name `%s' "
-					   "(%u chars max)",
-					   chain, XT_EXTENSION_MAXNAMELEN - 1);
-
-			policy = strtok(NULL, " \t\n");
-			DEBUGP("line %u, policy '%s'\n", line, policy);
-			if (!policy)
-				xtables_error(PARAMETER_PROBLEM,
-					   "%s: line %u policy invalid\n",
-					   xt_params->program_name, line);
-
-			if (nft_chain_builtin_find(p->curtable, chain)) {
-				if (counters) {
-					char *ctrs;
-					ctrs = strtok(NULL, " \t\n");
-
-					if (!ctrs || !parse_counters(ctrs, &count))
-						xtables_error(PARAMETER_PROBLEM,
-							   "invalid policy counters "
-							   "for chain '%s'\n", chain);
-
-				}
-				if (cb->chain_set &&
-				    cb->chain_set(h, p->curtable->name,
-					          chain, policy, &count) < 0) {
-					xtables_error(OTHER_PROBLEM,
-						      "Can't set policy `%s'"
-						      " on `%s' line %u: %s\n",
-						      policy, chain, line,
-						      ops->strerror(errno));
-				}
-				DEBUGP("Setting policy of chain %s to %s\n",
-				       chain, policy);
-			} else if (cb->chain_restore(h, chain, p->curtable->name) < 0 &&
-				   errno != EEXIST) {
-				xtables_error(PARAMETER_PROBLEM,
-					      "cannot create chain "
-					      "'%s' (%s)\n", chain,
-					      strerror(errno));
-			} else if (h->family == NFPROTO_BRIDGE &&
-				   !ebt_set_user_chain_policy(h, p->curtable->name,
-							      chain, policy)) {
+			if (cb->chain_set &&
+			    cb->chain_set(h, p->curtable->name,
+					  chain, policy, &count) < 0) {
 				xtables_error(OTHER_PROBLEM,
 					      "Can't set policy `%s'"
 					      " on `%s' line %u: %s\n",
 					      policy, chain, line,
 					      ops->strerror(errno));
 			}
-			ret = 1;
-		} else if (p->in_table) {
-			int a;
-			char *pcnt = NULL;
-			char *bcnt = NULL;
-			char *parsestart = buffer;
-
-			/* reset the newargv */
-			newargc = 0;
-
-			add_argv(xt_params->program_name, 0);
-			add_argv("-t", 0);
-			add_argv(p->curtable->name, 0);
-
-			tokenize_rule_counters(&parsestart, &pcnt, &bcnt, line);
-			if (counters && pcnt && bcnt) {
-				add_argv("--set-counters", 0);
-				add_argv(pcnt, 0);
-				add_argv(bcnt, 0);
-			}
+			DEBUGP("Setting policy of chain %s to %s\n",
+			       chain, policy);
+		} else if (cb->chain_restore(h, chain, p->curtable->name) < 0 &&
+			   errno != EEXIST) {
+			xtables_error(PARAMETER_PROBLEM,
+				      "cannot create chain "
+				      "'%s' (%s)\n", chain,
+				      strerror(errno));
+		} else if (h->family == NFPROTO_BRIDGE &&
+			   !ebt_set_user_chain_policy(h, p->curtable->name,
+						      chain, policy)) {
+			xtables_error(OTHER_PROBLEM,
+				      "Can't set policy `%s'"
+				      " on `%s' line %u: %s\n",
+				      policy, chain, line,
+				      ops->strerror(errno));
+		}
+		ret = 1;
+	} else if (p->in_table) {
+		int a;
+		char *pcnt = NULL;
+		char *bcnt = NULL;
+		char *parsestart = buffer;
+
+		/* reset the newargv */
+		newargc = 0;
+
+		add_argv(xt_params->program_name, 0);
+		add_argv("-t", 0);
+		add_argv(p->curtable->name, 0);
+
+		tokenize_rule_counters(&parsestart, &pcnt, &bcnt, line);
+		if (counters && pcnt && bcnt) {
+			add_argv("--set-counters", 0);
+			add_argv(pcnt, 0);
+			add_argv(bcnt, 0);
+		}
+
+		add_param_to_argv(parsestart, line);
 
-			add_param_to_argv(parsestart, line);
+		DEBUGP("calling do_command4(%u, argv, &%s, handle):\n",
+			newargc, p->curtable->name);
 
-			DEBUGP("calling do_command4(%u, argv, &%s, handle):\n",
-				newargc, p->curtable->name);
+		for (a = 0; a < newargc; a++)
+			DEBUGP("argv[%u]: %s\n", a, newargv[a]);
 
-			for (a = 0; a < newargc; a++)
-				DEBUGP("argv[%u]: %s\n", a, newargv[a]);
+		ret = cb->do_command(h, newargc, newargv,
+				    &newargv[2], true);
+		if (ret < 0) {
+			if (cb->abort)
+				ret = cb->abort(h);
+			else
+				ret = 0;
 
-			ret = cb->do_command(h, newargc, newargv,
-					    &newargv[2], true);
 			if (ret < 0) {
-				if (cb->abort)
-					ret = cb->abort(h);
-				else
-					ret = 0;
-
-				if (ret < 0) {
-					fprintf(stderr, "failed to abort "
-							"commit operation\n");
-				}
-				exit(1);
+				fprintf(stderr, "failed to abort "
+						"commit operation\n");
 			}
-
-			free_argv();
-			fflush(stdout);
-		}
-		if (p->tablename && p->curtable &&
-		    (strcmp(p->tablename, p->curtable->name) != 0))
-			continue;
-		if (!ret) {
-			fprintf(stderr, "%s: line %u failed\n",
-					xt_params->program_name, line);
 			exit(1);
 		}
+
+		free_argv();
+		fflush(stdout);
+	}
+	if (p->tablename && p->curtable &&
+	    (strcmp(p->tablename, p->curtable->name) != 0))
+		return;
+	if (!ret) {
+		fprintf(stderr, "%s: line %u failed\n",
+				xt_params->program_name, line);
+		exit(1);
+	}
+}
+
+void xtables_restore_parse(struct nft_handle *h,
+			   struct nft_xt_restore_parse *p,
+			   struct nft_xt_restore_cb *cb)
+{
+	char buffer[10240];
+
+	line = 0;
+
+	if (!h->noflush)
+		nft_fake_cache(h);
+	else
+		nft_build_cache(h);
+
+	/* Grab standard input. */
+	while (fgets(buffer, sizeof(buffer), p->in)) {
+		h->error.lineno = ++line;
+		xtables_restore_parse_line(h, p, cb, buffer);
 	}
 	if (p->in_table && p->commit) {
 		fprintf(stderr, "%s: COMMIT expected at line %u\n",
-- 
2.23.0