S"timefmt" flag to newsyslog(8)

[hrs]

Summary

This patch adds S"timefmt" flag to newsyslog(8). This allows to specify
a time format used in filenames of the rotated log files instead of
sequential ones on a per-file basis.

Test Plan

The following config file entry will trim /var/log/a.log and create
/var/log/a.log-20190820T032934.bz2, for example:

/var/log/a.log 644 2 1000 * JS"%Y%m%dT%H%M%S"C

[bcr]

A few minor additions to the man page.
I like this change, thanks for implementing it!

usr.sbin/newsyslog/newsyslog.8

274–279

s/default each/default, each/

282–287

Same here as above.

usr.sbin/newsyslog/newsyslog.conf.5

326

s/to/to the/

[hrs]

Updated to include bcr's suggestions and fix a missing length check.

[bcr]

Good to go!

[jilles]

usr.sbin/newsyslog/newsyslog.c

171

It would be more consistent with the rest of the values here to create a separate allocation for the string (char *suffix). This would also avoid the need to depend on the maximum path length, which I like to avoid if it is easy, since there are situations (not here) where MAXPATHLEN is too short.

1316

q cannot be NULL here and in the below new lines, particularly because it has just been incremented.

1317

Allowing ' as well as " (undocumented) seems unnecessary extra complexity, especially because a ' can end a string starting with " and vice versa.

1332

Suggest memcpy, or overwriting the terminating " with a '\0' (as occurs earlier in this function) and strdup (combined with my above remark about the type of struct conf_entry.suffix). Even though it is correct here due to the above check, the dstsize parameter to strlcpy should be the destination buffer's size; it should not be abused to truncate a string. Note that strlcpy calls strlen on src.

1759

I can see some benefit in a hard-coded maximum length here.

[hrs]

Update based on jilles's comments:

• Dynamic memory allocation of suffix.
• Fix a bug in NULL check of q.
• Use memcpy() to get a substring.
• Accept ["] only, not ['].

[hrs]

usr.sbin/newsyslog/newsyslog.c

171

I think the (theoretical) maximum length of the suffix is limited to MAXPATHLEN because it is eventually used as a filename. Can you elaborate the case where MAXPATHLEN is too short?

1759

datetimestr is an expanded string from the suffix and the total length is also shorter than MAXPATHLEN. Why is a 30-byte array specified in a numeric value better?

[jilles]

usr.sbin/newsyslog/newsyslog.c

171

Our value of 1024 for MAXPATHLEN is less than Linux's value of 4096 and therefore people sometimes do things where our value is too low. As I said, that's not an issue in this particular case.

1759

I considered MAXPATHLEN hard-coded too, but in any case it should be long enough for a date and time.