From: "Alvar Penning" Subject: Re: newsyslog: support glob(3) To: "Stuart Henderson" Cc: "Jan Klemkow" , Date: Mon, 16 Mar 2026 21:22:02 +0100 Hi Stuart, On Sat Mar 14, 2026 at 1:39 AM CET, Stuart Henderson wrote: > I would quite like to have this feature and it seems a pretty clean > implementation. I have some small nits inline n the diff, but there's > one slight issue that is a bit trickier. Thanks for your reply and your input. In the following new diff, I have addressed these comments. > A convenient way to use this would be to archive all files in a dir, > e.g. /var/log/consoles/*. However, previously archived files are > candidates for rotation too. Say you have rotation at 250K and an > existing file that's much larger than this, every run of newsyslog > it will now get rotated again, > > somelog.1.gz > somelog.1.gz.0.gz > somelog.1.gz.0.gz.0.gz > somelog.1.gz.0.gz.0.gz.0.gz > somelog.1.gz.0.gz.0.gz.0.gz.0.gz > somelog.1.gz.0.gz.0.gz.0.gz.0.gz.0.gz > somelog.1.gz.0.gz.0.gz.0.gz.0.gz.0.gz.0.gz > > and on until you hit PATH_MAX. > > On the one hand, the user gets exactly what they ask for, but on the > other, avoiding this is difficult with a simple glob. (I don't really > want regexps in config either though). Any idea how to avoid this? > Would it make sense to just skip files with the typical archival > suffixes automatically? I remember thinking about this when I first wrote these changes. But then went on using a pattern like "/somelog*.log" and putting these concerns under "user configuration issues". However, this is definitely a footgun. You are totally right. In the new diff, newsyslog checks if a globbed path has a suffix indicating this file is a rotation, also taking the 'Z' compression flag into account. Please feel free to take another look and thanks for the feedback. Best, Alvar diff --git a/newsyslog.8 b/newsyslog.8 index 391c7c3d99d..e57552b304c 100644 --- a/newsyslog.8 +++ b/newsyslog.8 @@ -180,6 +180,9 @@ follows: .Bl -tag -width XXXXXXXXXXXXXXXX .It Ar logfile_name The full pathname of the system log file to be archived. +This path might be a +.Xr glob 7 +pattern, which will be expanded accordingly. .It Ar owner:group This optional field specifies the owner and group for the archive file. The @@ -442,6 +445,7 @@ default configuration file .Sh SEE ALSO .Xr compress 1 , .Xr gzip 1 , +.Xr glob 7 , .Xr syslog 3 , .Xr syslogd 8 .Sh AUTHORS diff --git a/newsyslog.c b/newsyslog.c index 0be4ed259b9..e5ecc3a5316 100644 --- a/newsyslog.c +++ b/newsyslog.c @@ -88,6 +88,7 @@ #include #include #include +#include #include #include #include @@ -161,6 +162,8 @@ int stat_suffix(char *, size_t, char *, struct stat *, int (*)(const char *, struct stat *)); off_t sizefile(struct stat *); int parse_file(struct entrylist *, int *); +int is_glob_rotation(struct conf_entry *, const char *); +int parse_entry(struct entrylist *, int *, int, struct conf_entry *); time_t parse8601(char *); time_t parseDWM(char *); void child_killer(int); @@ -470,13 +473,14 @@ int parse_file(struct entrylist *list, int *nentries) { char line[BUFSIZ], *parse, *q, *errline, *group, *tmp, *ep; - struct conf_entry *working; - struct stat sb; + struct conf_entry *working, *working_glob; + size_t i; int lineno = 0; int ret = 0; FILE *f; const char *errstr; long l; + glob_t g; if (strcmp(conf, "-") == 0) f = stdin; @@ -504,9 +508,6 @@ nextline: if (working->log == NULL) err(1, NULL); - if ((working->logbase = strrchr(working->log, '/')) != NULL) - working->logbase++; - q = parse = missing_field(sob(++parse), errline, lineno); *(parse = son(parse)) = '\0'; if ((group = strchr(q, ':')) != NULL || @@ -739,57 +740,144 @@ nextline: goto nextline; } - /* 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, NULL); - *(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; + if (glob(working->log, GLOB_NOCHECK | GLOB_NOSORT, NULL, &g)) { + warnx("%s:%d: cannot glob(3), %s" + " --> skipping", + conf, lineno, strerror(errno)); + ret = 1; + goto nextline; + } - /* Make sure we can't oflow PATH_MAX */ - if (working->backdir != NULL) { - if (snprintf(line, sizeof(line), "%s/%s.%d%s", - working->backdir, working->logbase, - working->numlogs, COMPRESS_POSTFIX) >= PATH_MAX) { - warnx("%s:%d: pathname too long: %s" - " --> skipping", conf, lineno, q); - ret = 1; - goto nextline; + for (i = 0; i < g.gl_pathc; i++) { + DPRINTF(("%s:%d: expanded logfile name \"%s\" to \"%s\"" + " (%zu/%zu)\n", + conf, lineno, working->log, g.gl_pathv[i], + i+1, g.gl_pathc)); + + if (is_glob_rotation(working, g.gl_pathv[i])) { + DPRINTF(("%s:%d: \"%s\" looks like a rotation" + " --> skipping\n", + conf, lineno, g.gl_pathv[i])); + continue; } - } else { - if (snprintf(line, sizeof(line), "%s.%d%s", - working->log, working->numlogs, COMPRESS_POSTFIX) - >= PATH_MAX) { - warnx("%s:%d: pathname too long: %s" - " --> skipping", conf, lineno, - working->log); + + working_glob = malloc(sizeof(*working)); + if (working_glob == NULL) + err(1, NULL); + + memcpy(working_glob, working, sizeof(*working)); + + if ((working_glob->log = strdup(g.gl_pathv[i])) == NULL) + err(1, NULL); + + if (parse_entry(list, nentries, lineno, working_glob)) { ret = 1; - goto nextline; + /* break to free resources below */ + break; } } - TAILQ_INSERT_TAIL(list, working, next); - (*nentries)++; + + /* + * Original working is duplicated into working_glob in the for + * loop above and can be freed now. Every struct char pointer + * next to log is identical in each duplicate and used later. + */ + free(working->log); + free(working); + + globfree(&g); } (void)fclose(f); return (ret); } +int +is_glob_rotation(struct conf_entry *working, const char *glob_line) +{ + size_t endpos = strlen(glob_line) - 1; + + /* Ignore empty inputs or identical to the logfile name. */ + if (*glob_line == '\0' || strcmp(working->log, glob_line) == 0) + return 0; + + /* Suffix check for compressed files. */ + if (working->flags & CE_COMPACT && endpos >= strlen(COMPRESS_POSTFIX)) { + if (strcmp( + glob_line + endpos - (strlen(COMPRESS_POSTFIX) - 1), + COMPRESS_POSTFIX + ) != 0) + return 0; + + endpos -= strlen(COMPRESS_POSTFIX); + } + + /* Expect at least one digit. */ + if (endpos == 0 || !isdigit(glob_line[endpos--])) + return 0; + + for (; endpos > 0 && isdigit(glob_line[endpos]); endpos--); + + /* Expect dot before digits. */ + if (endpos == 0 || glob_line[endpos] != '.') + return 0; + + return 1; +} + +int +parse_entry( + struct entrylist *list, int *nentries, + int lineno, struct conf_entry *working +) +{ + char line[BUFSIZ]; + int linelen; + struct stat sb; + + if ((working->logbase = strrchr(working->log, '/')) != NULL) + working->logbase++; + + /* 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, NULL); + *(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 PATH_MAX */ + if (working->backdir != NULL) + linelen = snprintf(line, sizeof(line), "%s/%s.%d%s", + working->backdir, working->logbase, + working->numlogs, COMPRESS_POSTFIX); + else + linelen = snprintf(line, sizeof(line), "%s.%d%s", + working->log, working->numlogs, COMPRESS_POSTFIX); + if (linelen < 0 || linelen >= sizeof(line) || linelen >= PATH_MAX) { + warnx("%s:%d: pathname too long: %s --> skipping", conf, lineno, line); + return 1; + } + + TAILQ_INSERT_TAIL(list, working, next); + (*nentries)++; + return 0; +} + char * missing_field(char *p, char *errline, int lineno) {