From: Alvar Penning Subject: Re: newsyslog: support glob(3) through new G flag To: tech@openbsd.org Date: Sat, 1 Jun 2024 14:40:26 +0200 I would be happy if someone took another look. Thanks a lot. On 2024-04-26 15:35, Alvar Penning wrote: > On 2024-04-24 19:12, "Todd C. Miller" wrote: > > Do we really need yet another flag bit for this? If we want to > > support globbing it should be enough to check for glob characters > > in logfile_name and call glob(3) if they are present. > > Thanks for your comment. This seems fair enough. I have attached an > updated diff with this change. > > Best, > Alvar > > > diff --git usr.bin/newsyslog/newsyslog.8 usr.bin/newsyslog/newsyslog.8 > index 9e3f84a0b26..4dd95cb8e12 100644 > --- usr.bin/newsyslog/newsyslog.8 > +++ usr.bin/newsyslog/newsyslog.8 > @@ -170,7 +170,7 @@ By default, this configuration file is > Each line of the file contains information about a particular log > file that should be handled by > .Nm newsyslog . > -Each line has five mandatory fields and up to three optional fields, > with +Each line has five mandatory fields and up to four optional > fields, with whitespace separating each field. > Blank lines or lines beginning with a hash mark > .Pq Ql # > @@ -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 > @@ -359,8 +362,13 @@ rotate on every 5th day of the month at 6:00 hr > .It Ar flags > The optional > .Ar flags > -field specifies if the archives should have any special processing > -done to the archived log files. > +field specifies any special processing for each entry. > +Multiple > +.Ar flags > +can be concatenated together. > +.Pp > +.Bl -tag -width Ds -compact -offset indent > +.It Li Z > The > .Sq Z > flag will make the archive > @@ -368,7 +376,9 @@ files compressed to save space using > .Xr gzip 1 > or > .Xr compress 1 , > -depending on compilation options. > +depending on compilation options, defaulting to > +.Xr gzip 1 . > +.It Li B > The > .Sq B > flag means that the file is a > @@ -376,13 +386,16 @@ binary file, and so the ASCII message which > .Nm > inserts to indicate the fact that the logs have been turned over > should not be included. > +.It Li M > The > .Sq M > flag marks this entry as a monitored > log file. > +.It Li F > The > .Sq F > flag specifies that symbolic links should be followed. > +.El > .It Ar monitor > Specify the username (or email address) that should receive > notification messages if this is a monitored log file. > @@ -434,6 +447,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 usr.bin/newsyslog/newsyslog.c usr.bin/newsyslog/newsyslog.c > index 852f1dfac87..3fbf230d5a4 100644 > --- usr.bin/newsyslog/newsyslog.c > +++ usr.bin/newsyslog/newsyslog.c > @@ -88,6 +88,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -160,6 +161,7 @@ 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 parse_entry(struct entrylist *, int *, int, struct conf_entry *); > time_t parse8601(char *); > time_t parseDWM(char *); > void child_killer(int); > @@ -471,12 +473,13 @@ 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; > long l; > + glob_t g; > > if (strcmp(conf, "-") == 0) > f = stdin; > @@ -504,9 +507,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 || > @@ -717,57 +717,111 @@ 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; > - > - /* 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); > + if (strpbrk(working->log, "*?[")) { > + /* > + * This case also matches for escaped glob > characters. However, the > + * glob(3) function itself honors escape > characters, thus resulting > + * to a globed file list with only the > desired logfile. > + */ > + if (glob(working->log, GLOB_NOSORT, NULL, > &g)) { > + warnx("%s:%d: cannot glob(3), %s --> > skipping", > + conf, lineno, > strerror(errno)); ret = 1; > goto nextline; > } > - } 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); > + DPRINTF(("%s:%d: expanded glob %s to %zu > files\n", > + conf, lineno, working->log, g.gl_pathc)); > + > + for (i = 0; i < g.gl_pathc; i++) { > + 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; > + } > + } > + > + /* > + * It is not necessary to duplicate and free > the other three char > + * pointers in conf_entry as those values > are identical for each > + * globed file. Even more important, later > on they will only be > + * referenced in a reading fashion. > + */ > + free(working->log); > + free(working); > + > + globfree(&g); > + } else > + if (parse_entry(list, nentries, lineno, > working)) { ret = 1; > goto nextline; > } > - } > - TAILQ_INSERT_TAIL(list, working, next); > - (*nentries)++; > } > (void)fclose(f); > return (ret); > } > > +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 >= 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) > { >