On Fri, 14 Jul 2017 22:59:58 +0200, Jeremie Courreges-Anglas wrote:
> "Todd C. Miller" <todd.mil...@courtesan.com> writes:
>
> > On Fri, 14 Jul 2017 22:15:49 +0200, Jeremie Courreges-Anglas wrote:
> >
> >> the proposal in
> >>
> >> https://marc.info/?l=openbsd-misc&m=150001966325010&w=2
> >>
> >> made me look at newsyslog and its manually linked lists. Here's a stab
> >> at using TAILQs instead.
> >
> > Oh good, now I don't have to do it ;-)
>
> ;)
>
> > I have a diff to make change errx() into warnx() + continue.
>
> I have a similar diff which is a bit intrusive, as it tries to properly
> free resources allocated in parse_file(). Maybe show us yours first?
This doesn't attempt to free any resources. One thing I wanted to
do was to make parse_file() return an error value when there was a
parse error so the exit code could be non-zero.
- todd
Index: usr.bin/newsyslog/newsyslog.c
===================================================================
RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.c,v
retrieving revision 1.103
diff -u -p -u -r1.103 newsyslog.c
--- usr.bin/newsyslog/newsyslog.c 14 Jul 2017 20:51:17 -0000 1.103
+++ usr.bin/newsyslog/newsyslog.c 14 Jul 2017 21:11:29 -0000
@@ -510,9 +510,11 @@ parse_file(struct entrylist *list, int *
*group++ = '\0';
if (*q) {
if (!(isnumberstr(q))) {
- if ((pwd = getpwnam(q)) == NULL)
- errx(1, "%s:%d: unknown user:
%s",
+ if ((pwd = getpwnam(q)) == NULL) {
+ warnx("%s:%d: unknown user: %s",
conf, lineno, q);
+ continue;
+ }
working->uid = pwd->pw_uid;
} else
working->uid = atoi(q);
@@ -522,9 +524,11 @@ parse_file(struct entrylist *list, int *
q = group;
if (*q) {
if (!(isnumberstr(q))) {
- if ((grp = getgrnam(q)) == NULL)
- errx(1, "%s:%d: unknown group:
%s",
+ if ((grp = getgrnam(q)) == NULL) {
+ warnx("%s:%d: unknown group:
%s",
conf, lineno, q);
+ continue;
+ }
working->gid = grp->gr_gid;
} else
working->gid = atoi(q);
@@ -539,15 +543,19 @@ parse_file(struct entrylist *list, int *
}
l = strtol(q, &ep, 8);
- if (*ep != '\0' || l < 0 || l > ALLPERMS)
- errx(1, "%s:%d: bad permissions: %s", conf, lineno, q);
+ if (*ep != '\0' || l < 0 || l > ALLPERMS) {
+ warnx("%s:%d: bad permissions: %s", conf, lineno, q);
+ continue;
+ }
working->permissions = (mode_t)l;
q = parse = missing_field(sob(++parse), errline, lineno);
*(parse = son(parse)) = '\0';
l = strtol(q, &ep, 10);
- if (*ep != '\0' || l < 0 || l >= INT_MAX)
- errx(1, "%s:%d: bad number: %s", conf, lineno, q);
+ if (*ep != '\0' || l < 0 || l >= INT_MAX) {
+ warnx("%s:%d: bad number: %s", conf, lineno, q);
+ continue;
+ }
working->numlogs = (int)l;
q = parse = missing_field(sob(++parse), errline, lineno);
@@ -561,23 +569,29 @@ parse_file(struct entrylist *list, int *
q = parse = missing_field(sob(++parse), errline, lineno);
*(parse = son(parse)) = '\0';
l = strtol(q, &ep, 10);
- if (l < 0 || l >= INT_MAX)
- errx(1, "%s:%d: interval out of range: %s", conf,
+ if (l < 0 || l >= INT_MAX) {
+ warnx("%s:%d: interval out of range: %s", conf,
lineno, q);
+ continue;
+ }
working->hours = (int)l;
switch (*ep) {
case '\0':
break;
case '@':
working->trim_at = parse8601(ep + 1);
- if (working->trim_at == (time_t) - 1)
- errx(1, "%s:%d: bad time: %s", conf, lineno, q);
+ if (working->trim_at == (time_t) - 1) {
+ warnx("%s:%d: bad time: %s", conf, lineno, q);
+ continue;
+ }
working->flags |= CE_TRIMAT;
break;
case '$':
working->trim_at = parseDWM(ep + 1);
- if (working->trim_at == (time_t) - 1)
- errx(1, "%s:%d: bad time: %s", conf, lineno, q);
+ if (working->trim_at == (time_t) - 1) {
+ warnx("%s:%d: bad time: %s", conf, lineno, q);
+ continue;
+ }
working->flags |= CE_TRIMAT;
break;
case '*':
@@ -585,8 +599,8 @@ parse_file(struct entrylist *list, int *
break;
/* FALLTHROUGH */
default:
- errx(1, "%s:%d: bad interval/at: %s", conf, lineno, q);
- break;
+ warnx("%s:%d: bad interval/at: %s", conf, lineno, q);
+ continue;
}
q = sob(++parse); /* Optional field */
@@ -612,9 +626,9 @@ parse_file(struct entrylist *list, int *
working->flags |= CE_FOLLOW;
break;
default:
- errx(1, "%s:%d: illegal flag: `%c'",
+ warnx("%s:%d: illegal flag: `%c'",
conf, lineno, *q);
- break;
+ continue;
}
q++;
}
@@ -631,9 +645,11 @@ parse_file(struct entrylist *list, int *
break;
if (*q == '/') {
*(parse = son(parse)) = '\0';
- if (strlen(q) >= PATH_MAX)
- errx(1, "%s:%d: pathname too long: %s",
+ if (strlen(q) >= PATH_MAX) {
+ warnx("%s:%d: pathname too long: %s",
conf, lineno, q);
+ continue;
+ }
working->pidfile = strdup(q);
if (working->pidfile == NULL)
err(1, "strdup");
@@ -656,23 +672,29 @@ parse_file(struct entrylist *list, int *
break;
}
}
- if (i == NSIG)
- errx(1, "%s:%d: unknown signal: %s",
+ if (i == NSIG) {
+ warnx("%s:%d: unknown signal: %s",
conf, lineno, q);
+ continue;
+ }
} else if (working->flags & CE_MONITOR) {
*(parse = son(parse)) = '\0';
working->whom = strdup(q);
if (working->whom == NULL)
err(1, "strdup");
- } else
- errx(1, "%s:%d: unrecognized field: %s",
+ } else {
+ warnx("%s:%d: unrecognized field: %s",
conf, lineno, q);
+ continue;
+ }
}
free(errline);
- if ((working->flags & CE_MONITOR) && working->whom == NULL)
- errx(1, "%s:%d: missing monitor notification field",
+ if ((working->flags & CE_MONITOR) && working->whom == NULL) {
+ warnx("%s:%d: missing monitor notification field",
conf, lineno);
+ continue;
+ }
/* If there is an arcdir, set working->backdir. */
if (arcdir != NULL && working->logbase != NULL) {
@@ -701,15 +723,19 @@ parse_file(struct entrylist *list, int *
if (working->backdir != NULL) {
if (snprintf(line, sizeof(line), "%s/%s.%d%s",
working->backdir, working->logbase,
- working->numlogs, COMPRESS_POSTFIX) >= PATH_MAX)
- errx(1, "%s:%d: pathname too long: %s",
+ working->numlogs, COMPRESS_POSTFIX) >= PATH_MAX) {
+ warnx("%s:%d: pathname too long: %s",
conf, lineno, q);
+ continue;
+ }
} else {
if (snprintf(line, sizeof(line), "%s.%d%s",
working->log, working->numlogs, COMPRESS_POSTFIX)
- >= PATH_MAX)
- errx(1, "%s:%d: pathname too long: %s",
+ >= PATH_MAX) {
+ warnx("%s:%d: pathname too long: %s",
conf, lineno, working->log);
+ continue;
+ }
}
TAILQ_INSERT_TAIL(list, working, next);
(*nentries)++;
