Download raw body.
smtpd: unify the table(5) parser with makemap
Looks good to me, two comments inline.
- todd
> diff /home/op/tmp/smtpd
> commit - 1f95d9310cff81a3ddd7b95069ea5dea8d1b03de
> path + /home/op/tmp/smtpd
> blob - 2144e83959e9c25b24fc0913b0907a5af86a3804
> file + makemap.c
> --- makemap.c
> +++ makemap.c
> @@ -37,9 +37,8 @@
>
> static void usage(void);
> static int parse_map(DB *, int *, char *);
> -static int parse_entry(DB *, int *, char *, size_t, size_t);
> -static int parse_mapentry(DB *, int *, char *, size_t, size_t);
> -static int parse_setentry(DB *, int *, char *, size_t, size_t);
> +static int add_mapentry(DB *, int *, char *, char *, size_t);
> +static int add_setentry(DB *, int *, char *, size_t);
> static int make_plain(DBT *, char *);
> static int make_aliases(DBT *, char *);
> static char *conf_aliases(char *);
> @@ -247,9 +246,10 @@ static int
> parse_map(DB *db, int *dbputs, char *filename)
> {
> FILE *fp;
> - char *line;
> - size_t len;
> + char *key, *val, *line = NULL;
> + size_t linesize;
You may want to set linesize=0 here for consistency. It is not
strictly required since line is NULL, but you do zero it out in the
other consumer of parse_table_line().
> size_t lineno = 0;
> + int malformed, table_type, r;
>
> if (strcmp(filename, "-") == 0)
> fp = fdopen(0, "r");
> @@ -269,56 +269,47 @@ parse_map(DB *db, int *dbputs, char *filename)
> return 0;
> }
>
> - while ((line = fparseln(fp, &len, &lineno,
> - NULL, FPARSELN_UNESCCOMM)) != NULL) {
> - if (!parse_entry(db, dbputs, line, len, lineno)) {
> + table_type = (type == T_SET) ? T_LIST : T_HASH;
> + while (parse_table_line(fp, &line, &linesize, &table_type,
> + &key, &val, &malformed) != -1) {
> + lineno++;
> + if (malformed) {
> + warnx("%s:%zd: invalid entry", source, lineno);
> free(line);
> fclose(fp);
> return 0;
> }
> - free(line);
> + if (key == NULL)
> + continue;
> +
> + switch (type) {
> + case T_PLAIN:
> + case T_ALIASES:
> + r = add_mapentry(db, dbputs, key, val, lineno);
> + break;
> + case T_SET:
> + r = add_setentry(db, dbputs, key, lineno);
> + break;
> + }
> +
> + if (!r) {
> + free(line);
> + fclose(fp);
> + return 0;
> + }
> }
>
> + free(line);
> fclose(fp);
> return 1;
> }
>
> static int
> -parse_entry(DB *db, int *dbputs, char *line, size_t len, size_t lineno)
> +add_mapentry(DB *db, int *dbputs, char *keyp, char *valp, size_t lineno)
> {
> - switch (type) {
> - case T_PLAIN:
> - case T_ALIASES:
> - return parse_mapentry(db, dbputs, line, len, lineno);
> - case T_SET:
> - return parse_setentry(db, dbputs, line, len, lineno);
> - }
> - return 0;
> -}
> -
> -static int
> -parse_mapentry(DB *db, int *dbputs, char *line, size_t len, size_t lineno)
> -{
> DBT key;
> DBT val;
> - char *keyp;
> - char *valp;
>
> - keyp = line;
> - while (isspace((unsigned char)*keyp))
> - keyp++;
> - if (*keyp == '\0')
> - return 1;
> -
> - valp = keyp;
> - strsep(&valp, " \t:");
> - if (valp == NULL || valp == keyp)
> - goto bad;
> - while (*valp == ':' || isspace((unsigned char)*valp))
> - valp++;
> - if (*valp == '\0')
> - goto bad;
> -
> /* Check for dups. */
> key.data = keyp;
> key.size = strlen(keyp) + 1;
> @@ -355,18 +346,11 @@ bad:
> }
>
> static int
> -parse_setentry(DB *db, int *dbputs, char *line, size_t len, size_t lineno)
> +add_setentry(DB *db, int *dbputs, char *keyp, size_t lineno)
> {
> DBT key;
> DBT val;
> - char *keyp;
>
> - keyp = line;
> - while (isspace((unsigned char)*keyp))
> - keyp++;
> - if (*keyp == '\0')
> - return 1;
> -
> val.data = "<set>";
> val.size = strlen(val.data) + 1;
>
> blob - 25090c3acf9980c0fbe22784a5242dae64c8ec53
> file + smtpd.h
> --- smtpd.h
> +++ smtpd.h
> @@ -1738,6 +1738,9 @@ void log_trace0(const char *, ...)
> __attribute__((format (printf, 1, 2)));
> #define log_trace(m, ...) do { if (tracing & (m)) log_trace0(__VA_ARGS__);
> } while (0)
>
> +int parse_table_line(FILE *, char **, size_t *, int *,
> + char **, char **, int *);
> +
> /* waitq.c */
> int waitq_wait(void *, void (*)(void *, void *, void *), void *);
> void waitq_run(void *, void *);
> blob - af57038f2313f787ad14ab9156a97a2487df87b4
> file + table_static.c
> --- table_static.c
> +++ table_static.c
> @@ -111,80 +111,28 @@ static int
> table_static_priv_load(struct table_static_priv *priv, const char *path)
> {
> FILE *fp;
> - char *buf = NULL, *p;
> + char *line = NULL;
> int lineno = 0;
> - size_t sz = 0;
> - ssize_t flen;
> + size_t linesize = 0;
> char *keyp;
> char *valp;
> - int ret = 0;
> + int malformed, ret = 0;
>
> if ((fp = fopen(path, "r")) == NULL) {
> log_warn("%s: fopen", path);
> return 0;
> }
>
> - while ((flen = getline(&buf, &sz, fp)) != -1) {
> + while (parse_table_line(fp, &line, &linesize, &priv->type,
> + &keyp, &valp, &malformed) != -1) {
> lineno++;
> - if (buf[flen - 1] == '\n')
> - buf[--flen] = '\0';
> -
> - keyp = buf;
> - while (isspace((unsigned char)*keyp)) {
> - ++keyp;
> - --flen;
> - }
> - if (*keyp == '\0')
> - continue;
> - while (isspace((unsigned char)keyp[flen - 1]))
> - keyp[--flen] = '\0';
> - if (*keyp == '#') {
> - if (priv->type == T_NONE) {
> - keyp++;
> - while (isspace((unsigned char)*keyp))
> - ++keyp;
> - if (!strcmp(keyp, "@list"))
> - priv->type = T_LIST;
> - }
> - continue;
> - }
> -
> - if (priv->type == T_NONE) {
> - for (p = keyp; *p; p++) {
> - if (*p == ' ' || *p == '\t' || *p == ':') {
> - priv->type = T_HASH;
> - break;
> - }
> - }
> - if (priv->type == T_NONE)
> - priv->type = T_LIST;
> - }
> -
> - if (priv->type == T_LIST) {
> - table_static_priv_add(priv, keyp, NULL);
> - continue;
> - }
> -
> - /* T_HASH */
> - valp = keyp;
> - strsep(&valp, " \t:");
> - if (valp) {
> - while (*valp) {
> - if (!isspace((unsigned char)*valp) &&
> - !(*valp == ':' &&
> - isspace((unsigned char)*(valp + 1))))
> - break;
> - ++valp;
> - }
> - if (*valp == '\0')
> - valp = NULL;
> - }
> - if (valp == NULL) {
> - log_warnx("%s: invalid map entry line %d",
> + if (malformed) {
> + log_warnx("%s:%d invalid map entry",
> path, lineno);
> goto end;
> }
> -
> + if (keyp == NULL)
> + continue;
> table_static_priv_add(priv, keyp, valp);
> }
>
> @@ -199,7 +147,7 @@ table_static_priv_load(struct table_static_priv *priv,
>
> ret = 1;
> end:
> - free(buf);
> + free(line);
> fclose(fp);
> return ret;
> }
> blob - feb663cc61ec2572b57741fd694926f0c26967c7
> file + util.c
> --- util.c
> +++ util.c
> @@ -826,3 +826,75 @@ log_trace_verbose(int v)
> /* Set debug logging in log.c */
> log_setverbose(v & TRACE_DEBUG ? 2 : foreground_log);
> }
> +
> +int
> +parse_table_line(FILE *fp, char **line, size_t *linesize,
> + int *type, char **key, char **val, int *malformed)
> +{
> + char *keyp, *valp, *p;
> + ssize_t linelen;
> +
> + *key = NULL;
> + *val = NULL;
> + *malformed = 0;
> +
> + if ((linelen = getline(line, linesize, fp)) == -1)
> + return (-1);
> +
> + keyp = *line;
> + while (isspace((unsigned char)*keyp)) {
> + ++keyp;
> + --linelen;
> + }
> + if (*keyp == '\0')
> + return 0;
> + while (isspace((unsigned char)keyp[linelen - 1]))
> + keyp[--linelen] = '\0';
I would feel better if this was:
while (linelen > 0 && isspace((unsigned char)keyp[linelen - 1]))
keyp[--linelen] = '\0';
I realize it should not be possible for linelen to go negative here
because you've already trimmed leading space, but it doesn't hurt
to be extra careful. It will probably also save you from a static
analyzer complaint.
> + if (*keyp == '#') {
> + if (*type == T_NONE) {
> + keyp++;
> + while (isspace((unsigned char)*keyp))
> + ++keyp;
> + if (!strcmp(keyp, "@list"))
> + *type = T_LIST;
> + }
> + return 0;
> + }
> +
> + if (*type == T_NONE) {
> + for (p = keyp; *p; p++) {
> + if (*p == ' ' || *p == '\t' || *p == ':') {
> + *type = T_HASH;
> + break;
> + }
> + }
> + if (*type == T_NONE)
> + *type = T_LIST;
> + }
> +
> + if (*type == T_LIST) {
> + *key = keyp;
> + return (0);
> + }
> +
> + /* T_HASH */
> + valp = keyp;
> + strsep(&valp, " \t:");
> + if (valp) {
> + while (*valp) {
> + if (!isspace((unsigned char)*valp) &&
> + !(*valp == ':' &&
> + isspace((unsigned char)*(valp + 1))))
> + break;
> + ++valp;
> + }
> + if (*valp == '\0')
> + valp = NULL;
> + }
> + if (valp == NULL)
> + *malformed = 1;
> +
> + *key = keyp;
> + *val = valp;
> + return (0);
> +}
>
smtpd: unify the table(5) parser with makemap