I would be happy if someone took another look. Thanks a lot.

On 2024-04-26 15:35, Alvar Penning <post@0x21.biz> wrote:
> On 2024-04-24 19:12, "Todd C. Miller" <Todd.Miller@sudo.ws> 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 <err.h>
>  #include <errno.h>
>  #include <fcntl.h>
> +#include <glob.h>
>  #include <grp.h>
>  #include <limits.h>
>  #include <pwd.h>
> @@ -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)
>  {
>