Merge pull request #35 from ifupdown-ng/feature/bsearch-parsing

interface config parser rewrite

cmd/ifquery.c

@@ -275,6 +275,8 @@ ifquery_main(int argc, char *argv[])
 	struct lif_dict state = {};
 	struct lif_dict collection = {};
 
+	lif_interface_collection_init(&collection);
+
 	if (!lif_state_read_path(&state, exec_opts.state_file))
 	{
 		fprintf(stderr, "%s: could not parse %s\n", argv0, exec_opts.state_file);

cmd/ifupdown.c

@@ -163,6 +163,8 @@ ifupdown_main(int argc, char *argv[])
 	struct lif_dict state = {};
 	struct lif_dict collection = {};
 
+	lif_interface_collection_init(&collection);
+
 	if (!lif_state_read_path(&state, exec_opts.state_file))
 	{
 		fprintf(stderr, "%s: could not parse %s\n", argv0, exec_opts.state_file);

libifupdown/interface-file.c

@@ -14,6 +14,7 @@
  */
 
 #include <ctype.h>
+#include <stdarg.h>
 #include <stdio.h>
 #include <string.h>
 #include "libifupdown/libifupdown.h"
@@ -57,156 +58,92 @@ maybe_remap_token(const char *token)
 	return tokbuf;
 }
 
-bool
-lif_interface_file_parse(struct lif_dict *collection, const char *filename)
+/* XXX: remove this global variable somehow */
+static struct lif_interface *cur_iface = NULL;
+
+static void
+report_error(const char *filename, size_t lineno, const char *errfmt, ...)
 {
-	lif_interface_collection_init(collection);
-	struct lif_interface *cur_iface = NULL;
+	char errbuf[4096];
 
-	FILE *f = fopen(filename, "r");
-	if (f == NULL)
+	va_list va;
+	va_start(va, errfmt);
+	vsnprintf(errbuf, sizeof errbuf, errfmt, va);
+	va_end(va);
+
+	fprintf(stderr, "%s:%zu: %s\n", filename, lineno, errbuf);
+}
+
+static bool
+handle_address(struct lif_dict *collection, const char *filename, size_t lineno, char *token, char *bufp)
+{
+	(void) collection;
+	(void) token;
+
+	char *addr = lif_next_token(&bufp);
+
+	if (cur_iface == NULL)
+	{
+		report_error(filename, lineno, "%s '%s' without interface", token, addr);
 		return false;
-
-	char linebuf[4096];
-	while (lif_fgetline(linebuf, sizeof linebuf, f) != NULL)
-	{
-		char *bufp = linebuf;
-		char *token = lif_next_token(&bufp);
-
-		if (!*token || !isalpha(*token))
-			continue;
-
-		if (!strcmp(token, "source"))
-		{
-			char *source_filename = lif_next_token(&bufp);
-			if (!*source_filename)
-				goto parse_error;
-
-			if (!strcmp(filename, source_filename))
-			{
-				fprintf(stderr, "%s: attempt to source %s would create infinite loop\n",
-					filename, source_filename);
-				goto parse_error;
 	}
 
-			lif_interface_file_parse(collection, source_filename);
-		}
-		else if (!strcmp(token, "auto"))
-		{
+	lif_interface_address_add(cur_iface, addr);
+
+	return true;
+}
+
+static bool
+handle_auto(struct lif_dict *collection, const char *filename, size_t lineno, char *token, char *bufp)
+{
+	(void) filename;
+	(void) lineno;
+	(void) token;
+
 	char *ifname = lif_next_token(&bufp);
 	if (!*ifname && cur_iface == NULL)
-				goto parse_error;
+		return false;
 	else
 	{
 		cur_iface = lif_interface_collection_find(collection, ifname);
 		if (cur_iface == NULL)
-					goto parse_error;
+			return false;
 	}
 
 	cur_iface->is_auto = true;
-		}
-		else if (!strcmp(token, "iface"))
-		{
-			char *ifname = lif_next_token(&bufp);
-			if (!*ifname)
-				goto parse_error;
+	return true;
+}
 
-			cur_iface = lif_interface_collection_find(collection, ifname);
-			if (cur_iface == NULL)
-				goto parse_error;
+static bool
+handle_gateway(struct lif_dict *collection, const char *filename, size_t lineno, char *token, char *bufp)
+{
+	(void) collection;
+	(void) token;
 
-			/* in original ifupdown config, we can have "inet loopback"
-			 * or "inet dhcp" or such to designate hints.  lets pick up
-			 * those hints here.
-			 */
-			char *token = lif_next_token(&bufp);
-			while (*token)
-			{
-				if (!strcmp(token, "dhcp"))
-					lif_interface_use_executor(cur_iface, "dhcp");
-				else if (!strcmp(token, "ppp"))
-					lif_interface_use_executor(cur_iface, "ppp");
-				else if (!strcmp(token, "inherits"))
-				{
-					token = lif_next_token(&bufp);
-
-					if (!*token)
-					{
-						fprintf(stderr, "%s: inherits without interface\n", filename);
-						goto parse_error;
-					}
-
-					if (!lif_interface_collection_inherit(cur_iface, collection, token))
-					{
-						fprintf(stderr, "%s: could not inherit %s\n", filename, token);
-						goto parse_error;
-					}
-				}
-
-				token = lif_next_token(&bufp);
-			}
-		}
-		else if (!strcmp(token, "use"))
-		{
-			char *executor = lif_next_token(&bufp);
-
-			if (cur_iface == NULL)
-			{
-				fprintf(stderr, "%s: use '%s' without interface\n", filename, executor);
-				goto parse_error;
-			}
-
-			/* pass requires as compatibility env vars to appropriate executors (bridge, bond) */
-			if (!strcmp(executor, "bridge"))
-				cur_iface->is_bridge = true;
-			else if (!strcmp(executor, "bond"))
-				cur_iface->is_bond = true;
-
-			lif_interface_use_executor(cur_iface, executor);
-		}
-		else if (!strcmp(token, "inherit"))
-		{
-			token = lif_next_token(&bufp);
-
-			if (!*token)
-			{
-				fprintf(stderr, "%s: inherits without interface\n", filename);
-				goto parse_error;
-			}
-
-			if (!lif_interface_collection_inherit(cur_iface, collection, token))
-			{
-				fprintf(stderr, "%s: could not inherit %s\n", filename, token);
-				goto parse_error;
-			}
-		}
-		else if (!strcmp(token, "address"))
-		{
 	char *addr = lif_next_token(&bufp);
 
 	if (cur_iface == NULL)
 	{
-				fprintf(stderr, "%s: address '%s' without interface\n", filename, addr);
-				goto parse_error;
-			}
-
-			lif_interface_address_add(cur_iface, addr);
-		}
-		else if (!strcmp(token, "gateway"))
-		{
-			char *addr = lif_next_token(&bufp);
-
-			if (cur_iface == NULL)
-			{
-				fprintf(stderr, "%s: gateway '%s' without interface\n", filename, addr);
-				goto parse_error;
+		report_error(filename, lineno, "%s '%s' without interface", token, addr);
+		return false;
 	}
 
 	lif_interface_use_executor(cur_iface, "static");
 	lif_dict_add(&cur_iface->vars, token, strdup(addr));
-		}
-		else if (cur_iface != NULL)
-		{
+
+	return true;
+}
+
+static bool
+handle_generic(struct lif_dict *collection, const char *filename, size_t lineno, char *token, char *bufp)
+{
+	(void) collection;
+	(void) filename;
+	(void) lineno;
+
+	if (cur_iface == NULL)
+		return true;
+
 	token = maybe_remap_token(token);
 
 	lif_dict_add(&cur_iface->vars, token, strdup(bufp));
@@ -219,22 +156,178 @@ lif_interface_file_parse(struct lif_dict *collection, const char *filename)
 		char *addon = strndup(token, word_end - token);
 		lif_interface_use_executor(cur_iface, addon);
 		free(addon);
+	}
 
-				/* pass requires as compatibility env vars to appropriate executors (bridge, bond) */
-				if (!strcmp(addon, "bridge"))
-					cur_iface->is_bridge = true;
-				else if (!strcmp(addon, "bond"))
-					cur_iface->is_bond = true;
+	return true;
+}
+
+static bool handle_inherit(struct lif_dict *collection, const char *filename, size_t lineno, char *token, char *bufp);
+
+static bool
+handle_iface(struct lif_dict *collection, const char *filename, size_t lineno, char *token, char *bufp)
+{
+	char *ifname = lif_next_token(&bufp);
+	if (!*ifname)
+	{
+		report_error(filename, lineno, "%s without any other tokens", token);
+		return false;
 	}
+
+	cur_iface = lif_interface_collection_find(collection, ifname);
+	if (cur_iface == NULL)
+	{
+		report_error(filename, lineno, "could not upsert interface %s", ifname);
+		return false;
 	}
+
+	/* in original ifupdown config, we can have "inet loopback"
+	 * or "inet dhcp" or such to designate hints.  lets pick up
+	 * those hints here.
+	 */
+	token = lif_next_token(&bufp);
+	while (*token)
+	{
+		if (!strcmp(token, "dhcp"))
+			lif_interface_use_executor(cur_iface, "dhcp");
+		else if (!strcmp(token, "ppp"))
+			lif_interface_use_executor(cur_iface, "ppp");
+		else if (!strcmp(token, "inherits"))
+		{
+			if (!handle_inherit(collection, filename, lineno, token, bufp))
+				return false;
+		}
+
+		token = lif_next_token(&bufp);
+	}
+
+	return true;
+}
+
+static bool
+handle_inherit(struct lif_dict *collection, const char *filename, size_t lineno, char *token, char *bufp)
+{
+	char *target = lif_next_token(&bufp);
+
+	if (cur_iface == NULL)
+	{
+		report_error(filename, lineno, "%s '%s' without interface", token, target);
+		return false;
+	}
+
+	if (!*target)
+	{
+		report_error(filename, lineno, "%s: unspecified interface");
+		return false;
+	}
+
+	if (!lif_interface_collection_inherit(cur_iface, collection, target))
+	{
+		report_error(filename, lineno, "could not inherit %s", target);
+		return false;
+	}
+
+	return true;
+}
+
+static bool
+handle_source(struct lif_dict *collection, const char *filename, size_t lineno, char *token, char *bufp)
+{
+	(void) token;
+
+	char *source_filename = lif_next_token(&bufp);
+	if (!*source_filename)
+	{
+		report_error(filename, lineno, "missing filename to source");
+		return false;
+	}
+
+	if (!strcmp(filename, source_filename))
+	{
+		report_error(filename, lineno, "attempt to source %s would create infinite loop",
+			     source_filename);
+		return false;
+	}
+
+	return lif_interface_file_parse(collection, source_filename);
+}
+
+static bool
+handle_use(struct lif_dict *collection, const char *filename, size_t lineno, char *token, char *bufp)
+{
+	(void) collection;
+
+	char *executor = lif_next_token(&bufp);
+
+	if (cur_iface == NULL)
+	{
+		report_error(filename, lineno, "%s '%s' without interface", token, executor);
+		return false;
+	}
+
+	lif_interface_use_executor(cur_iface, executor);
+	return true;
+}
+
+/* map keywords to parser functions */
+struct parser_keyword {
+	const char *token;
+	bool (*handle)(struct lif_dict *collection, const char *filename, size_t lineno, char *token, char *bufp);
+};
+
+static const struct parser_keyword keywords[] = {
+	{"address", handle_address},
+	{"auto", handle_auto},
+	{"gateway", handle_gateway},
+	{"iface", handle_iface},
+	{"inherit", handle_inherit},
+	{"source", handle_source},
+	{"use", handle_use},
+};
+
+static int
+keyword_cmp(const void *a, const void *b)
+{
+	const char *key = a;
+	const struct parser_keyword *token = b;
+
+	return strcmp(key, token->token);
+}
+
+bool
+lif_interface_file_parse(struct lif_dict *collection, const char *filename)
+{
+	FILE *f = fopen(filename, "r");
+	if (f == NULL)
+		return false;
+
+	char linebuf[4096];
+	size_t lineno = 0;
+	while (lif_fgetline(linebuf, sizeof linebuf, f) != NULL)
+	{
+		lineno++;
+
+		char *bufp = linebuf;
+		char *token = lif_next_token(&bufp);
+
+		if (!*token || !isalpha(*token))
+			continue;
+
+		const struct parser_keyword *parserkw =
+			bsearch(token, keywords, ARRAY_SIZE(keywords), sizeof(*keywords), keyword_cmp);
+
+		if (parserkw != NULL)
+		{
+			if (!parserkw->handle(collection, filename, lineno, token, bufp))
+				goto parse_error;
+		}
+		else if (!handle_generic(collection, filename, lineno, token, bufp))
+			goto parse_error;
 	}
 
 	fclose(f);
 	return true;
 
 parse_error:
-	fprintf(stderr, "libifupdown: %s: failed to parse line \"%s\"\n",
-		filename, linebuf);
 	fclose(f);
 	return false;
 }

libifupdown/interface.c

@@ -141,6 +141,12 @@ lif_interface_use_executor(struct lif_interface *interface, const char *executor
 
 	if (lif_dict_add_once(&interface->vars, "use", exec_addon, (lif_dict_cmp_t) strcmp) == NULL)
 		free(exec_addon);
+
+	/* pass requires as compatibility env vars to appropriate executors (bridge, bond) */
+	if (!strcmp(executor, "bridge"))
+		interface->is_bridge = true;
+	else if (!strcmp(executor, "bond"))
+		interface->is_bond = true;
 }
 
 void