Download raw body.
newsyslog: support glob(3)
Am Mon, Mar 16, 2026 at 09:22:02PM +0100 schrieb Alvar Penning:
> 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.
Hi Alvar,
Thanks for the diff.
I tested the globbing with recursive filepathes and works for me.
Some comments see below.
Greeds Nico
> diff --git a/newsyslog.c b/newsyslog.c
> index 0be4ed259b9..e5ecc3a5316 100644
> --- a/newsyslog.c
> +++ b/newsyslog.c
> @@ -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)
according to style(9) it looks a bit odd
should be something like this:
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
> +)
same here:
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)
> {
>
newsyslog: support glob(3)