Configuration parser cleanup

- Do a getline() & process in rdline instead of doing a read per character
   and feeding it through a state machine.
 - Move splitkv to rdcfg.c, the only place where it is used

Signed-off-by: David Oberhollenzer <david.oberhollenzer@tele2.at>

cmd/runsvc/env.c

@@ -41,7 +41,7 @@ static struct entry *parse_list(rdline_t *rd)
 	char *ptr;
 
 	while (rdline(rd) == 0) {
-		ptr = rd->buffer;
+		ptr = rd->line;
 
 		while (*ptr != '\0' && *ptr != ' ' && *ptr != '=')
 			++ptr;
@@ -66,11 +66,11 @@ static struct entry *parse_list(rdline_t *rd)
 			continue;
 		}
 
-		e = calloc(1, sizeof(*e) + strlen(rd->buffer) + 1);
+		e = calloc(1, sizeof(*e) + strlen(rd->line) + 1);
 		if (e == NULL)
 			goto fail_oom;
 
-		strcpy(e->data, rd->buffer);
+		strcpy(e->data, rd->line);
 		e->next = list;
 		list = e;
 	}

lib/Makemodule.am

@@ -8,8 +8,7 @@ libinit_a_SOURCES += lib/util/print_version.c lib/util/argv_exec.c $(HEADRS)
 libinit_a_CPPFLAGS = $(AM_CPPFLAGS)
 libinit_a_CFLAGS = $(AM_CFLAGS)
 
-libcfg_a_SOURCES = lib/libcfg/rdline.c lib/libcfg/unescape.c
+libcfg_a_SOURCES = lib/libcfg/rdline.c lib/libcfg/unescape.c lib/libcfg/rdcfg.c
-libcfg_a_SOURCES += lib/libcfg/splitkv.c lib/libcfg/rdcfg.c
 libcfg_a_SOURCES += lib/libcfg/pack_argv.c lib/include/libcfg.h
 libcfg_a_CPPFLAGS = $(AM_CPPFLAGS)
 libcfg_a_CFLAGS = $(AM_CFLAGS)

lib/cron/rdcron.c

@@ -486,7 +486,8 @@ crontab_t *rdcron(int dirfd, const char *filename)
 	cron = calloc(1, sizeof(*cron));
 	if (cron == NULL) {
 		fputs("out of memory\n", stderr);
-		goto out;
+		close(fd);
+		return NULL;
 	}
 
 	cron->minute = 0xFFFFFFFFFFFFFFFFUL;
@@ -501,7 +502,6 @@ crontab_t *rdcron(int dirfd, const char *filename)
 		delcron(cron);
 		cron = NULL;
 	}
-out:
+	rdline_cleanup(&rd);
-	close(fd);
 	return cron;
 }

lib/include/libcfg.h

@@ -20,23 +20,16 @@
 
 #include <stdbool.h>
 #include <stddef.h>
+#include <stdio.h>
 
 typedef struct {
-	int fd;			/* input file descriptor */
-	const char *argstr;	/* if not NULL, read from this instead */
-
 	const char *filename;	/* input file name */
 	size_t lineno;		/* current line number */
-
+	FILE *fp;
-	size_t i;		/* buffer offset */
+	char *line;
-	char buffer[256];	/* current line, null-terminated */
 
 	int argc;
 	const char *const *argv;
-
-	bool string;		/* inside a string? */
-	bool escape;		/* reading an escape sequence? */
-	bool comment;		/* inside a comment */
 } rdline_t;
 
 typedef struct {
@@ -63,6 +56,8 @@ typedef struct {
 void rdline_init(rdline_t *t, int fd, const char *filename,
 		 int argc, const char *const *argv);
 
+void rdline_cleanup(rdline_t *t);
+
 /*
 	Read from file until end-of-file or a line feed is encountered.
 
@@ -84,9 +79,8 @@ void rdline_init(rdline_t *t, int fd, const char *filename,
 	   outside the bounds set by argc, processing fails. On success,
 	   the argv value is inserted and processed as described above.
 	 - A '%' character can be escaped by writing '%%' or, if inside
-	   a double quite string, by writing \%.
+	   a double quoted string, by writing \%.
-	 - An attempt to use such an indexed argument inside an argument
+	 - Arguments are pasted as is. Substitution is not recursive.
-	   expansion, results in failure.
 	 - If the resulting line is empty, processing is restarted.
 */
 int rdline(rdline_t *t);
@@ -110,15 +104,6 @@ int unescape(char *src);
 */
 int pack_argv(char *str);
 
-/*
-	Split the current input line into a space seperted keyword
-	(alphabetical characters only) and a value (the rest of the line).
-
-	If errors are encounted, prints a diagnostic message to stderr and
-	returns -1. On success, zero is returned.
- */
-int splitkv(rdline_t *rd, char **k, char **v);
-
 /*
 	Parse a configuration file containing '<keyword> [arguments...]' lines.
 	The cfgobj and flags are passed to the callback in the params array.

lib/libcfg/rdcfg.c

@@ -19,6 +19,7 @@
 
 #include <string.h>
 #include <stdio.h>
+#include <ctype.h>
 
 static const cfg_param_t *find_param(rdline_t *rd, const char *name,
 				     const cfg_param_t *params, size_t count)
@@ -35,6 +36,33 @@ static const cfg_param_t *find_param(rdline_t *rd, const char *name,
 	return NULL;
 }
 
+static int splitkv(rdline_t *rd, char **k, char **v)
+{
+	char *key = rd->line, *value = rd->line;
+
+	while (*value != ' ' && *value != '\0') {
+		if (!isalpha(*value)) {
+			fprintf(stderr,
+				"%s: %zu: unexpected '%c' in keyword\n",
+				rd->filename, rd->lineno, *value);
+			return -1;
+		}
+		++value;
+	}
+
+	if (*value != ' ') {
+		fprintf(stderr, "%s: %zu: expected argument after '%s'\n",
+			rd->filename, rd->lineno, key);
+		return -1;
+	}
+
+	*(value++) = '\0';
+
+	*k = key;
+	*v = value;
+	return 0;
+}
+
 int rdcfg(void *cfgobj, rdline_t *rd, const cfg_param_t *params, size_t count,
 	  int flags)
 {
@@ -61,9 +89,9 @@ int rdcfg(void *cfgobj, rdline_t *rd, const cfg_param_t *params, size_t count,
 			}
 
 			while ((ret = rdline(rd)) == 0) {
-				if (strcmp(rd->buffer, "}") == 0)
+				if (strcmp(rd->line, "}") == 0)
 					break;
-				if (p->handle(cfgobj, rd->buffer, rd, flags))
+				if (p->handle(cfgobj, rd->line, rd, flags))
 					return -1;
 			}
 

lib/libcfg/rdline.c

@@ -24,151 +24,144 @@
 
 #include "libcfg.h"
 
-static int rdline_getc(rdline_t *t)
-{
-	int ret;
-	char c;
-
-	if (t->argstr != NULL) {
-		c = *(t->argstr++);
-		if (c != '\0')
-			goto out;
-
-		t->argstr = NULL;
-	}
-
-	do {
-		ret = read(t->fd, &c, 1);
-	} while (ret < 0 && errno == EINTR);
-
-	if (ret < 0)
-		return -1;
-
-	if (ret == 0) {
-		if (t->i == 0) {
-			errno = 0;
-			return -1;
-		}
-		c = '\0';
-	}
-out:
-	return (c == '\n') ? '\0' : c;
-}
-
-static int rdline_append(rdline_t *t, int c)
-{
-	if (t->comment) {
-		if (c != '\0')
-			return 0;
-	} else if (t->string) {
-		if (t->escape) {
-			t->escape = false;
-		} else {
-			if (c == '\\')
-				t->escape = true;
-			if (c == '"')
-				t->string = false;
-		}
-	} else {
-		if (isspace(c))
-			c = ' ';
-		if (c == ' ' && (t->i == 0 || t->buffer[t->i - 1] == ' '))
-			return 0;
-		if (c == '#') {
-			t->comment = true;
-			return 0;
-		}
-		if (c == '"')
-			t->string = true;
-	}
-
-	if (c == '\0') {
-		while (t->i > 0 && t->buffer[t->i - 1] == ' ')
-			t->i -= 1;
-	}
-
-	if (t->i == sizeof(t->buffer))
-		return -1;
-
-	t->buffer[t->i++] = c;
-	return 0;
-}
-
 void rdline_init(rdline_t *t, int fd, const char *filename,
 		 int argc, const char *const *argv)
 {
 	memset(t, 0, sizeof(*t));
-	t->fd = fd;
+	t->fp = fdopen(fd, "r");
 	t->filename = filename;
 	t->argc = argc;
 	t->argv = argv;
 }
 
-int rdline(rdline_t *t)
+void rdline_cleanup(rdline_t *t)
 {
-	const char *errstr;
+	free(t->line);
-	int c;
+	fclose(t->fp);
-retry:
+}
-	t->i = 0;
+
-	t->argstr = NULL;
+static int read_raw_line(rdline_t *t)
-	t->string = t->escape = t->comment = false;
+{
+	size_t len = 0;
+
+	free(t->line);
+	t->line = NULL;
+
+	errno = 0;
+
+	if (getline(&t->line, &len, t->fp) < 0) {
+		if (errno) {
+			fprintf(stderr, "%s: %zu: %s\n", t->filename,
+				t->lineno, strerror(errno));
+			return -1;
+		}
+		return 1;
+	}
+
 	t->lineno += 1;
-
-	do {
-		errno = 0;
-		c = rdline_getc(t);
-		if (c < 0) {
-			if (errno == 0)
-				return 1;
-			errstr = strerror(errno);
-			goto fail;
-		}
-		if (c == 0 && t->string) {
-			errstr = "missing \"";
-			goto fail;
-		}
-
-		if (c == '%') {
-			c = rdline_getc(t);
-			if (c == 0) {
-				errstr = "unexpected end of line after '%%'";
-				goto fail;
-			}
-			if (c < 0) {
-				errstr = strerror(errno);
-				goto fail;
-			}
-
-			if (c != '%') {
-				if (!isdigit(c)) {
-					errstr = "exptected digit after '%%'";
-					goto fail;
-				}
-				if ((c - '0') >= t->argc) {
-					errstr = "argument out of range";
-					goto fail;
-				}
-				if (t->argstr != NULL) {
-					errstr = "recursive argument "
-						 "expansion";
-					goto fail;
-				}
-				t->argstr = t->argv[c - '0'];
-				continue;
-			}
-		}
-
-		if (rdline_append(t, c)) {
-			errstr = "line too long";
-			goto fail;
-		}
-	} while (c != '\0');
-
-	if (t->buffer[0] == '\0')
-		goto retry;
-
 	return 0;
+}
+
+static int normalize_line(rdline_t *t)
+{
+	char *dst = t->line, *src = t->line;
+	bool string = false;
+	const char *errstr;
+	int c, ret = 0;
+
+	while (isspace(*src))
+		++src;
+
+	while (*src != '\0' && (string || *src != '#')) {
+		c = *(src++);
+
+		if (c == '"') {
+			string = !string;
+		} else if (!string && isspace(c)) {
+			c = ' ';
+			if (dst > t->line && dst[-1] == ' ')
+				continue;
+		} else if (c == '%') {
+			*(dst++) = c;
+			c = *(src++);
+			if (c != '%' && !isdigit(c)) {
+				errstr = "expected digit after '%%'";
+				goto fail;
+			}
+			if (isdigit(c) && (c - '0') >= t->argc) {
+				errstr = "argument out of range";
+				goto fail;
+			}
+			ret += strlen(t->argv[c - '0']);
+		} else if (string && c == '\\' && *src != '\0') {
+			*(dst++) = c;
+			c = *(src++);
+		}
+
+		*(dst++) = c;
+	}
+
+	if (string) {
+		errstr = "missing \"";
+		goto fail;
+	}
+
+	while (dst > t->line && dst[-1] == ' ')
+		--dst;
+	*dst = '\0';
+	return ret;
 fail:
 	fprintf(stderr, "%s: %zu: %s\n", t->filename, t->lineno, errstr);
 	return -1;
 }
+
+static void substitute(rdline_t *t, char *dst, char *src)
+{
+	bool string = false;
+
+	while (*src != '\0') {
+		if (src[0] == '%' && isdigit(src[1])) {
+			strcpy(dst, t->argv[src[1] - '0']);
+			src += 2;
+			while (*dst != '\0')
+				++dst;
+		} else {
+			if (*src == '"')
+				string = !string;
+			if (string && *src == '\\')
+				*(dst++) = *(src++);
+			*(dst++) = *(src++);
+		}
+	}
+}
+
+int rdline(rdline_t *t)
+{
+	char *buffer = NULL;
+	int ret;
+
+	do {
+		if ((ret = read_raw_line(t)))
+			goto out;
+		if ((ret = normalize_line(t)) < 0)
+			goto out;
+	} while (t->line[0] == '\0');
+
+	if (ret == 0)
+		return 0;
+
+	buffer = calloc(1, strlen(t->line) + ret + 1);
+	if (buffer == NULL) {
+		fprintf(stderr, "%s: %zu: out of memory\n",
+			t->filename, t->lineno);
+		ret = -1;
+		goto out;
+	}
+
+	substitute(t, buffer, t->line);
+	ret = 0;
+out:
+	free(t->line);
+	t->line = buffer;
+	return ret;
+}

lib/libcfg/splitkv.c

@@ -1,48 +0,0 @@
-/* SPDX-License-Identifier: GPL-3.0-or-later */
-/*
- * Copyright (C) 2018 - David Oberhollenzer
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <https://www.gnu.org/licenses/>.
- */
-#include <ctype.h>
-#include <stdio.h>
-
-#include "libcfg.h"
-
-int splitkv(rdline_t *rd, char **k, char **v)
-{
-	char *key = rd->buffer, *value = rd->buffer;
-
-	while (*value != ' ' && *value != '\0') {
-		if (!isalpha(*value)) {
-			fprintf(stderr,
-				"%s: %zu: unexpected '%c' in keyword\n",
-				rd->filename, rd->lineno, *value);
-			return -1;
-		}
-		++value;
-	}
-
-	if (*value != ' ') {
-		fprintf(stderr, "%s: %zu: expected argument after '%s'\n",
-			rd->filename, rd->lineno, key);
-		return -1;
-	}
-
-	*(value++) = '\0';
-
-	*k = key;
-	*v = value;
-	return 0;
-}

lib/util/rdsvc.c

@@ -266,8 +266,6 @@ service_t *rdsvc(int dirfd, const char *filename, int flags)
 		argc = 0;
 	}
 
-	rdline_init(&rd, fd, filename, argc, args);
-
 	nlen = (arg != NULL) ? (size_t)(arg - filename) : strlen(filename);
 
 	svc = calloc(1, sizeof(*svc) + nlen + 1);
@@ -282,14 +280,17 @@ service_t *rdsvc(int dirfd, const char *filename, int flags)
 
 	memcpy(svc->name, filename, nlen);
 
-	if (rdcfg(svc, &rd, svc_params, ARRAY_SIZE(svc_params), flags))
+	rdline_init(&rd, fd, filename, argc, args);
-		goto fail;
 
-	close(fd);
+	if (rdcfg(svc, &rd, svc_params, ARRAY_SIZE(svc_params), flags)) {
+		delsvc(svc);
+		svc = NULL;
+	}
+
+	rdline_cleanup(&rd);
 	return svc;
 fail_oom:
 	fputs("out of memory\n", stderr);
-fail:
 	delsvc(svc);
 	close(fd);
 	return NULL;