Proposed patch fixes a very small typo in the newsyslog.conf(5) manpage that has the 'P' flag lowercased.
Created attachment 250961 [details] Patch to fix the case of the 'P' flag in newsyslog.conf's manpage
Lower case 'p' is correct it seems, from newsyslog.c: #define CE_PLAIN0 0x0800 /* Do not compress zero'th history file */ case 'p': working->flags |= CE_PLAIN0; break;
(In reply to Ed Maste from comment #2) The switch statement is changing the flag characters to lower case before it checks them: > for (; q && *q && !isspacech(*q); q++) { > switch (tolowerch(*q)) { > case 'b': > working->flags |= CE_BINARY; > break; > case 'c': > working->flags |= CE_CREATE; > break; In the man page, 'p' is the only config flag that's lowercase, which is why I am assuming that it's the typo. If it's the other way around and lowercase is supposed to be the correct format, then there's fourteen typos for the other uppercased flags that have gone unnoticed for a long time instead of one :)
(In reply to Joshua Kinard from comment #3) Why is it always the little things that hide the deepest of rabbit holes? I dug back into FreeBSD's cgit history, and it looks like the 'p'/'P' flag (take your pick) was added on 2017-12-31 by @eadler, tracked by git commit df76ac99518d. In that commit, the flag was added as lowercase to the newsyslog.conf.5 man page, and to the newsyslog.c source file as lower case. I then went and looked into NetBSD's source, and they have ALL of the newsyslog.conf flags they support listed in their man page as lowercase, but in their newsyslog.c, they convert those flags to uppercase before checking them in a switch statement, which is the opposite of what FreeBSD is doing. Digging further back, I think it's around NetBSD 1.6 is when support for those flags even first appeared, including 'p'. In that version, in newsyslog.c, NetBSD checks for lowercase flags. On 2007-21-12. when NetBSD was at v5.0, the 'J' flag was added to NetBSD's newsyslog.c by @dogcow, and at that time is when they modified the switch logic to check for uppercase flags, but added the 'J' flag to the man page as lowercase 'j', and did not leave a reasoning on these differences in their commit message: https://anonhg.netbsd.org/src/rev/e97bbfc29eff Which is correct? Both, technically! Historically, lowercase letters were first, in both NetBSD's man pages and source. Then the source later got updated to check the flags as uppercase values, but the man page still, to this day, references lower case. FreeBSD, on the other hand, is using uppercase flags in its newsyslog.conf(5) man page and config examples, except for 'p', but checks the flags as all lower case, which was the original logic from NetBSD. I assume that when the code for the 'p' flag was copied from NetBSD and brought into FreeBSD, @eadler either didn't notice such a small nit, or didn't care. Both upper and lowercase flag values are technically valid, because in either OS, the values are forced into one or the other case before being checked. If y'all want, I have no problem leaving the flag in the man page as-is as a historical curiosity for someone to find again in the future. Or, the patch can be accepted and the flag changed to uppercase in FreeBSD's man page for consistency with the other flags.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=a8f86ecd6104b1f127b426be278a9031789b4413 commit a8f86ecd6104b1f127b426be278a9031789b4413 Author: Joshua Kinard <freebsd@kumba.dev> AuthorDate: 2024-05-25 20:52:11 +0000 Commit: Ed Maste <emaste@FreeBSD.org> CommitDate: 2024-05-27 01:34:14 +0000 newsyslog: Fix case of the 'P' flag in newsyslog.conf(5) PR: 279303 Reviewed-by: emaste Signed-off-by: Joshua Kinard <freebsd@kumba.dev> usr.sbin/newsyslog/newsyslog.conf.5 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Thanks for the thorough chase through the rabbit hole! I've merged the patch as it makes sense for the man page to be consistent.
There are two possible follow-on changes that could be made: - Add a note in the man page that the flags are case insensitive but by convention are typically uppercase. - Switch the switch statement to upper case for diff reduction against NetBSD, if the two versions of newvers.sh are still sufficiently similar otherwise.
(In reply to Ed Maste from comment #7) Seems straight-forward enough. I'll see about getting some patches ready this weekend.
Created attachment 251206 [details] [PATCH 1/2] Use uppercase for newsyslog.conf(5) flags parsing First of two patches to address the remaining nits for newsyslog.
Created attachment 251207 [details] [PATCH 2/2] Note that flags are case-insensitive, but typically uppercase Second of the two patches.