On 2014-12-17 17:44, Todd C. Miller wrote:
It would be helpful if you could break the diff up into smaller
logical pieces. For instance, one diff for the SLIST changes,
another for general cleanup and a third for the glob() changes.
- todd
Ok, my bad. I re-did the changes in stages using quilt and produced 4
different patches.
Any feedback is appreciated.
Seraphim
--- a/newsyslog.c
+++ b/newsyslog.c
@@ -91,6 +91,7 @@
#endif
#include <sys/param.h>
+#include <sys/queue.h>
#include <sys/stat.h>
#include <sys/time.h>
#include <sys/wait.h>
@@ -138,8 +139,10 @@ struct conf_entry {
char *whom; /* Whom to notify if logfile changes */
char *pidfile; /* Path to file containing pid to signal */
char *runcmd; /* Command to run instead of sending a signal */
- struct conf_entry *next; /* Linked list pointer */
+ SLIST_ENTRY(conf_entry) entries; /* SLIST struct */
};
+SLIST_HEAD(conflist, conf_entry);
+struct conflist all_confs = SLIST_HEAD_INITIALIZER(all_confs);
struct pidinfo {
char *file;
@@ -170,8 +173,7 @@ int movefile(char *, char *, uid_t, gid_
int stat_suffix(char *, size_t, char *, struct stat *,
int (*)(const char *, struct stat *));
off_t sizefile(struct stat *);
-struct conf_entry *
- parse_file(int *);
+void parse_file(int *);
time_t parse8601(char *);
time_t parseDWM(char *);
void child_killer(int);
@@ -187,8 +189,9 @@ void usage(void);
int
main(int argc, char **argv)
{
- struct conf_entry *p, *q, *x, *y;
+ struct conf_entry *q, *t;
struct pidinfo *pidlist, *pl;
+ struct conflist *p = &all_confs;
int status, listlen;
char **av;
@@ -199,30 +202,26 @@ main(int argc, char **argv)
if (needroot && getuid() && geteuid())
errx(1, "You must be root.");
- p = parse_file(&listlen);
+ parse_file(&listlen);
+
if (argc > 0) {
/* Only rotate specified files. */
- x = y = NULL;
- listlen = 0;
- for (av = argv; *av; av++) {
- for (q = p; q; q = q->next)
- if (strcmp(*av, q->log) == 0) {
- if (x == NULL)
- x = y = q;
- else {
- y->next = q;
- y = q;
- }
- listlen++;
- break;
- }
- if (q == NULL)
- warnx("%s: %s not found", conf, *av);
+ SLIST_FOREACH_SAFE(q, p, entries, t) {
+ for (av = argv; *av; av++) {
+ if (strcmp(*av, q->log) == 0)
+ goto next;
+ }
+ /* FIXME: Find a clever way to print the warning */
+ //warnx("%s: %s not found", conf, *av);
+ SLIST_REMOVE(p, q, conf_entry, entries);
+ free(q);
+ listlen--;
+next:
+ ;
}
- if (x == NULL)
+
+ if (SLIST_EMPTY(p))
errx(1, "%s: no specified log files", conf);
- y->next = NULL;
- p = x;
}
pidlist = (struct pidinfo *)calloc(listlen + 1, sizeof(struct pidinfo));
@@ -232,11 +231,12 @@ main(int argc, char **argv)
signal(SIGCHLD, child_killer);
/* Step 1, rotate all log files */
- for (q = p; q; q = q->next)
+ SLIST_FOREACH(q, p, entries)
do_entry(q);
/* Step 2, make a list of unique pid files */
- for (q = p, pl = pidlist; q; ) {
+ SLIST_FOREACH(q, p, entries) {
+ pl = pidlist;
if (q->flags & CE_ROTATED) {
struct pidinfo *pltmp;
@@ -259,7 +259,6 @@ main(int argc, char **argv)
pl++;
}
}
- q = q->next;
}
/* Step 3, send a signal or run a command */
@@ -275,12 +274,12 @@ main(int argc, char **argv)
sleep(5);
/* Step 4, compress the log.0 file if configured to do so and free */
- while (p) {
- if ((p->flags & CE_COMPACT) && (p->flags & CE_ROTATED) &&
- p->numlogs > 0)
- compress_log(p);
- q = p;
- p = p->next;
+ while (!SLIST_EMPTY(p)) {
+ q = SLIST_FIRST(p);
+ if ((q->flags & CE_COMPACT) && (q->flags & CE_ROTATED) &&
+ q->numlogs > 0)
+ compress_log(q);
+ SLIST_REMOVE_HEAD(p, entries);
free(q);
}
@@ -473,11 +472,11 @@ usage(void)
* Parse a configuration file and return a linked list of all the logs
* to process
*/
-struct conf_entry *
+void
parse_file(int *nentries)
{
char line[BUFSIZ], *parse, *q, *errline, *group, *tmp, *ep;
- struct conf_entry *working = NULL, *first = NULL;
+ struct conf_entry *working = NULL;
struct passwd *pwd;
struct group *grp;
struct stat sb;
@@ -499,18 +498,12 @@ parse_file(int *nentries)
if (errline == NULL)
err(1, "strdup");
(*nentries)++;
- if (!first) {
- working = malloc(sizeof(struct conf_entry));
- if (working == NULL)
- err(1, "malloc");
- first = working;
- } else {
- working->next = malloc(sizeof(struct conf_entry));
- if (working->next == NULL)
- err(1, "malloc");
- working = working->next;
- }
+ working = malloc(sizeof(struct conf_entry));
+ if (working == NULL)
+ err(1, "malloc");
+
+ SLIST_INSERT_HEAD(&all_confs, working, entries);
q = parse = missing_field(sob(line), errline, lineno);
*(parse = son(line)) = '\0';
working->log = strdup(q);
@@ -729,10 +722,7 @@ parse_file(int *nentries)
conf, lineno, working->log);
}
}
- if (working)
- working->next = NULL;
(void)fclose(f);
- return (first);
}
char *
--- a/newsyslog.c
+++ b/newsyslog.c
@@ -1059,8 +1059,7 @@ update:
cleanup:
free(flog);
- if (rb != NULL)
- free(rb);
+ free(rb);
return (1);
}
--- a/newsyslog.c
+++ b/newsyslog.c
@@ -194,7 +194,7 @@ main(int argc, char **argv)
struct conflist *p = &all_confs;
int status, listlen;
char **av;
-
+
parse_args(argc, argv);
argc -= optind;
argv += optind;
@@ -528,7 +528,7 @@ parse_file(int *nentries)
working->uid = atoi(q);
} else
working->uid = (uid_t)-1;
-
+
q = group;
if (*q) {
if (!(isnumberstr(q))) {
@@ -540,7 +540,7 @@ parse_file(int *nentries)
working->gid = atoi(q);
} else
working->gid = (gid_t)-1;
-
+
q = parse = missing_field(sob(++parse), errline, lineno);
*(parse = son(parse)) = '\0';
} else {
@@ -566,7 +566,7 @@ parse_file(int *nentries)
working->size = atoi(q) * 1024;
else
working->size = -1;
-
+
working->flags = 0;
q = parse = missing_field(sob(++parse), errline, lineno);
*(parse = son(parse)) = '\0';
@@ -1026,7 +1026,7 @@ domonitor(struct conf_entry *ent)
fclose(fp);
goto cleanup;
}
-
+
/* Send message. */
fclose(fp);
--- a/newsyslog.c
+++ b/newsyslog.c
@@ -100,6 +100,7 @@
#include <err.h>
#include <errno.h>
#include <fcntl.h>
+#include <glob.h>
#include <grp.h>
#include <limits.h>
#include <pwd.h>
@@ -174,6 +175,8 @@ int stat_suffix(char *, size_t, char *,
int (*)(const char *, struct stat *));
off_t sizefile(struct stat *);
void parse_file(int *);
+struct conf_entry *
+ parse_line(char *, char *, char *, int);
time_t parse8601(char *);
time_t parseDWM(char *);
void child_killer(int);
@@ -475,14 +478,11 @@ usage(void)
void
parse_file(int *nentries)
{
- char line[BUFSIZ], *parse, *q, *errline, *group, *tmp, *ep;
+ char line[BUFSIZ], *parse, *errline, *tmp, *q;
struct conf_entry *working = NULL;
- struct passwd *pwd;
- struct group *grp;
- struct stat sb;
- int lineno;
+ int lineno, i;
FILE *f;
- long l;
+ glob_t g;
if (strcmp(conf, "-") == 0)
f = stdin;
@@ -497,232 +497,254 @@ parse_file(int *nentries)
errline = strdup(tmp);
if (errline == NULL)
err(1, "strdup");
- (*nentries)++;
- working = malloc(sizeof(struct conf_entry));
- if (working == NULL)
- err(1, "malloc");
-
- SLIST_INSERT_HEAD(&all_confs, working, entries);
- q = parse = missing_field(sob(line), errline, lineno);
- *(parse = son(line)) = '\0';
- working->log = strdup(q);
- if (working->log == NULL)
- err(1, "strdup");
+ q = parse = missing_field(sob(line), errline, lineno);
+ *(parse = son(line)) = '\0';
+ parse++;
+ if(glob(q, GLOB_NOCHECK, NULL, &g) != 0)
+ err(1, "glob");
+
+ for (i=0; i < g.gl_matchc ; i++) {
+ (*nentries)++;
+ working = parse_line(g.gl_pathv[i], parse, errline, lineno);
+ SLIST_INSERT_HEAD(&all_confs, working, entries);
+ }
+ free(errline);
+ }
+ (void)fclose(f);
+}
- if ((working->logbase = strrchr(working->log, '/')) != NULL)
- working->logbase++;
+struct conf_entry *
+parse_line(char *logfile, char *rest, char *errline, int lineno)
+{
+ char *line, *parse, *q, *group, *tmp, *ep;
+ struct conf_entry *working = NULL;
+ struct passwd *pwd;
+ struct group *grp;
+ struct stat sb;
+ long l;
- q = parse = missing_field(sob(++parse), errline, lineno);
- *(parse = son(parse)) = '\0';
- if ((group = strchr(q, ':')) != NULL ||
- (group = strrchr(q, '.')) != NULL) {
- *group++ = '\0';
- if (*q) {
- if (!(isnumberstr(q))) {
- if ((pwd = getpwnam(q)) == NULL)
- errx(1, "%s:%d: unknown user: %s",
- conf, lineno, q);
- working->uid = pwd->pw_uid;
- } else
- working->uid = atoi(q);
+ working = malloc(sizeof(struct conf_entry));
+ if (working == NULL)
+ err(1, "malloc");
+
+ working->log = strdup(logfile);
+ if (working->log == NULL)
+ err(1, "strdup");
+
+ if ((working->logbase = strrchr(working->log, '/')) != NULL)
+ working->logbase++;
+
+ line = strdup(rest);
+ q = parse = missing_field(sob(line), errline, lineno);
+ *(parse = son(parse)) = '\0';
+ if ((group = strchr(q, ':')) != NULL ||
+ (group = strrchr(q, '.')) != NULL) {
+ *group++ = '\0';
+ if (*q) {
+ if (!(isnumberstr(q))) {
+ if ((pwd = getpwnam(q)) == NULL)
+ errx(1, "%s:%d: unknown user: %s",
+ conf, lineno, q);
+ working->uid = pwd->pw_uid;
} else
- working->uid = (uid_t)-1;
+ working->uid = atoi(q);
+ } else
+ working->uid = (uid_t)-1;
- q = group;
- if (*q) {
- if (!(isnumberstr(q))) {
- if ((grp = getgrnam(q)) == NULL)
- errx(1, "%s:%d: unknown group: %s",
- conf, lineno, q);
- working->gid = grp->gr_gid;
- } else
- working->gid = atoi(q);
+ q = group;
+ if (*q) {
+ if (!(isnumberstr(q))) {
+ if ((grp = getgrnam(q)) == NULL)
+ errx(1, "%s:%d: unknown group: %s",
+ conf, lineno, q);
+ working->gid = grp->gr_gid;
} else
- working->gid = (gid_t)-1;
-
- q = parse = missing_field(sob(++parse), errline, lineno);
- *(parse = son(parse)) = '\0';
- } else {
- working->uid = (uid_t)-1;
+ working->gid = atoi(q);
+ } else
working->gid = (gid_t)-1;
- }
-
- l = strtol(q, &ep, 8);
- if (*ep != '\0' || l < 0 || l > ALLPERMS)
- errx(1, "%s:%d: bad permissions: %s", conf, lineno, q);
- 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);
- working->numlogs = (int)l;
+ } else {
+ working->uid = (uid_t)-1;
+ working->gid = (gid_t)-1;
+ }
- q = parse = missing_field(sob(++parse), errline, lineno);
- *(parse = son(parse)) = '\0';
- if (isdigit((unsigned char)*q))
- working->size = atoi(q) * 1024;
- else
- working->size = -1;
+ l = strtol(q, &ep, 8);
+ if (*ep != '\0' || l < 0 || l > ALLPERMS)
+ errx(1, "%s:%d: bad permissions: %s", conf, lineno, q);
+ 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);
+ working->numlogs = (int)l;
+
+ q = parse = missing_field(sob(++parse), errline, lineno);
+ *(parse = son(parse)) = '\0';
+ if (isdigit((unsigned char)*q))
+ working->size = atoi(q) * 1024;
+ else
+ working->size = -1;
- working->flags = 0;
- 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,
- lineno, q);
- working->hours = (int)l;
- switch (*ep) {
- case '\0':
+ working->flags = 0;
+ 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,
+ lineno, q);
+ 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);
+ 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);
+ working->flags |= CE_TRIMAT;
+ break;
+ case '*':
+ if (q == ep)
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);
- 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);
- working->flags |= CE_TRIMAT;
- break;
- case '*':
- if (q == ep)
+ /* FALLTHROUGH */
+ default:
+ errx(1, "%s:%d: bad interval/at: %s", conf, lineno, q);
+ break;
+ }
+
+ q = sob(++parse); /* Optional field */
+ if (*q == 'Z' || *q == 'z' || *q == 'B' || *q == 'b' ||
+ *q == 'M' || *q == 'm') {
+ *(parse = son(q)) = '\0';
+ while (*q) {
+ switch (*q) {
+ case 'Z':
+ case 'z':
+ working->flags |= CE_COMPACT;
break;
- /* FALLTHROUGH */
- default:
- errx(1, "%s:%d: bad interval/at: %s", conf, lineno, q);
- break;
+ case 'B':
+ case 'b':
+ working->flags |= CE_BINARY;
+ break;
+ case 'M':
+ case 'm':
+ working->flags |= CE_MONITOR;
+ break;
+ case 'F':
+ case 'f':
+ working->flags |= CE_FOLLOW;
+ break;
+ default:
+ errx(1, "%s:%d: illegal flag: `%c'",
+ conf, lineno, *q);
+ break;
+ }
+ q++;
}
+ } else
+ parse--; /* no flags so undo */
- q = sob(++parse); /* Optional field */
- if (*q == 'Z' || *q == 'z' || *q == 'B' || *q == 'b' ||
- *q == 'M' || *q == 'm') {
- *(parse = son(q)) = '\0';
- while (*q) {
- switch (*q) {
- case 'Z':
- case 'z':
- working->flags |= CE_COMPACT;
- break;
- case 'B':
- case 'b':
- working->flags |= CE_BINARY;
- break;
- case 'M':
- case 'm':
- working->flags |= CE_MONITOR;
- break;
- case 'F':
- case 'f':
- working->flags |= CE_FOLLOW;
- break;
- default:
- errx(1, "%s:%d: illegal flag: `%c'",
- conf, lineno, *q);
- break;
- }
- q++;
+ working->pidfile = PIDFILE;
+ working->signal = SIGHUP;
+ working->runcmd = NULL;
+ working->whom = NULL;
+ for (;;) {
+ q = parse = sob(++parse); /* Optional field */
+ if (q == NULL || *q == '\0')
+ break;
+ if (*q == '/') {
+ *(parse = son(parse)) = '\0';
+ if (strlen(q) >= MAXPATHLEN)
+ errx(1, "%s:%d: pathname too long: %s",
+ conf, lineno, q);
+ working->pidfile = strdup(q);
+ if (working->pidfile == NULL)
+ err(1, "strdup");
+ } else if (*q == '"' && (tmp = strchr(q + 1, '"'))) {
+ *(parse = tmp) = '\0';
+ if (*++q != '\0') {
+ working->runcmd = strdup(q);
+ if (working->runcmd == NULL)
+ err(1, "strdup");
}
- } else
- parse--; /* no flags so undo */
+ working->pidfile = NULL;
+ working->signal = -1;
+ } else if (strncmp(q, "SIG", 3) == 0) {
+ int i;
- working->pidfile = PIDFILE;
- working->signal = SIGHUP;
- working->runcmd = NULL;
- working->whom = NULL;
- for (;;) {
- q = parse = sob(++parse); /* Optional field */
- if (q == NULL || *q == '\0')
- break;
- if (*q == '/') {
- *(parse = son(parse)) = '\0';
- if (strlen(q) >= MAXPATHLEN)
- errx(1, "%s:%d: pathname too long: %s",
- conf, lineno, q);
- working->pidfile = strdup(q);
- if (working->pidfile == NULL)
- err(1, "strdup");
- } else if (*q == '"' && (tmp = strchr(q + 1, '"'))) {
- *(parse = tmp) = '\0';
- if (*++q != '\0') {
- working->runcmd = strdup(q);
- if (working->runcmd == NULL)
- err(1, "strdup");
- }
- working->pidfile = NULL;
- working->signal = -1;
- } else if (strncmp(q, "SIG", 3) == 0) {
- int i;
-
- *(parse = son(parse)) = '\0';
- for (i = 1; i < NSIG; i++) {
- if (!strcmp(sys_signame[i], q + 3)) {
- working->signal = i;
- break;
- }
+ *(parse = son(parse)) = '\0';
+ for (i = 1; i < NSIG; i++) {
+ if (!strcmp(sys_signame[i], q + 3)) {
+ working->signal = i;
+ break;
}
- if (i == NSIG)
- errx(1, "%s:%d: unknown signal: %s",
- conf, lineno, q);
- } 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",
- conf, lineno, q);
- }
- free(errline);
-
- if ((working->flags & CE_MONITOR) && working->whom == NULL)
- errx(1, "%s:%d: missing monitor notification field",
- conf, lineno);
-
- /* If there is an arcdir, set working->backdir. */
- if (arcdir != NULL && working->logbase != NULL) {
- if (*arcdir == '/') {
- /* Fully qualified arcdir */
- working->backdir = arcdir;
- } else {
- /* arcdir is relative to log's parent dir */
- *(working->logbase - 1) = '\0';
- if ((asprintf(&working->backdir, "%s/%s",
- working->log, arcdir)) == -1)
- err(1, "malloc");
- *(working->logbase - 1) = '/';
- }
- /* Ignore arcdir if it doesn't exist. */
- if (stat(working->backdir, &sb) != 0 ||
- !S_ISDIR(sb.st_mode)) {
- if (working->backdir != arcdir)
- free(working->backdir);
- working->backdir = NULL;
}
+ if (i == NSIG)
+ errx(1, "%s:%d: unknown signal: %s",
+ conf, lineno, q);
+ } else if (working->flags & CE_MONITOR) {
+ *(parse = son(parse)) = '\0';
+ working->whom = strdup(q);
+ if (working->whom == NULL)
+ err(1, "strdup");
} else
- working->backdir = NULL;
+ errx(1, "%s:%d: unrecognized field: %s",
+ conf, lineno, q);
+ }
- /* Make sure we can't oflow MAXPATHLEN */
- if (working->backdir != NULL) {
- if (snprintf(line, sizeof(line), "%s/%s.%d%s",
- working->backdir, working->logbase,
- working->numlogs, COMPRESS_POSTFIX) >= MAXPATHLEN)
- errx(1, "%s:%d: pathname too long: %s",
- conf, lineno, q);
+ if ((working->flags & CE_MONITOR) && working->whom == NULL)
+ errx(1, "%s:%d: missing monitor notification field",
+ conf, lineno);
+
+ /* If there is an arcdir, set working->backdir. */
+ if (arcdir != NULL && working->logbase != NULL) {
+ if (*arcdir == '/') {
+ /* Fully qualified arcdir */
+ working->backdir = arcdir;
} else {
- if (snprintf(line, sizeof(line), "%s.%d%s",
- working->log, working->numlogs, COMPRESS_POSTFIX)
- >= MAXPATHLEN)
- errx(1, "%s:%d: pathname too long: %s",
- conf, lineno, working->log);
+ /* arcdir is relative to log's parent dir */
+ *(working->logbase - 1) = '\0';
+ if ((asprintf(&working->backdir, "%s/%s",
+ working->log, arcdir)) == -1)
+ err(1, "malloc");
+ *(working->logbase - 1) = '/';
+ }
+ /* Ignore arcdir if it doesn't exist. */
+ if (stat(working->backdir, &sb) != 0 ||
+ !S_ISDIR(sb.st_mode)) {
+ if (working->backdir != arcdir)
+ free(working->backdir);
+ working->backdir = NULL;
}
+ } else
+ working->backdir = NULL;
+
+ /* Make sure we can't oflow MAXPATHLEN */
+ if (working->backdir != NULL) {
+ if (snprintf(line, sizeof(line), "%s/%s.%d%s",
+ working->backdir, working->logbase,
+ working->numlogs, COMPRESS_POSTFIX) >= MAXPATHLEN)
+ errx(1, "%s:%d: pathname too long: %s",
+ conf, lineno, q);
+ } else {
+ if (snprintf(line, sizeof(line), "%s.%d%s",
+ working->log, working->numlogs, COMPRESS_POSTFIX)
+ >= MAXPATHLEN)
+ errx(1, "%s:%d: pathname too long: %s",
+ conf, lineno, working->log);
}
- (void)fclose(f);
+
+ return working;
}
char *
