On Thu, 22-Jan-2026 at 08:31:15 -0700, Warner Losh wrote:
> On Thu, Jan 22, 2026 at 1:46 AM Andre Albsmeier <[email protected]> wrote:
>
> > On Fri, 19-Dec-2025 at 20:37:41 +0000, Jamie Landeg-Jones wrote:
> > > void <[email protected]> wrote:
> > >
> > > >...
> > > > and so dmsg has things like
> > > >
> > > > [1353919] swp_pager_getswapspace(32): failed
> > > >
> > > > the [1353919] I guess being the timestamp.
> > > >
> > > > ...
> > > >
> > > Yes! You can get the value from sysctl kern.boottime.
> > >
> > > Here is a small program I wrote a while back, just call it
> > > as a dmesg substitute.
> > > ...
> > > Cheers, Jamie
> > >
> > > #!/bin/sh -efu
> > > set -efu
> > >
> > > boottime="$(/sbin/sysctl -n kern.boottime | /usr/local/bin/gawk '{printf
> > "%d", gensub ("^.* sec = ([1-9][0-9]*), .*$", "\\1", 1)}')"
> > >
> > > [ -z "$(printf '%s' "$boottime" | /usr/bin/egrep '^0$|^[1-9][0-9]*$')" ]
> > && { printf 'Invalid boottime retrieved.\n' >& 2; exit 1; }
> > >
> > > /sbin/dmesg "$@" | /usr/local/bin/gawk -v boottime="$boottime" '
> > >
> > > {
> > > uptime = gensub ("^\\[([1-9][0-9]*)\\] .*$", "\\1", 1)
> > > if (uptime == $0) realtime = "??? ?? ??:??;??"
> > > else realtime = strftime ("%b %d %T", uptime + boottime)
> > >
> > > print realtime " " $0
> > > }'
> >
> > FWIW, this is a patch for dmesg(1) I use. You can enable timestamp
> > conversion using -t and optionally specify a different output format
> > using -f "fmt".
> >
> > If interested for inclusion in base, I could try to re-format it
> > and come up with a manpage diff...
> >
>
> Yes. I've done a quick review inline.
Thanks...
> > --- sbin/dmesg/dmesg.c 2023-12-27 17:49:57.000000000 +0100
> > +++ sbin/dmesg/dmesg.c 2026-01-22 09:20:28.987020000 +0100
> > @@ -60,6 +60,7 @@
> > #include <unistd.h>
> > #include <vis.h>
> > #include <sys/syslog.h>
> > +#include <time.h>
> >
> > static struct nlist nl[] = {
> > #define X_MSGBUF 0
> > @@ -76,18 +77,23 @@
> > main(int argc, char *argv[])
> > {
> > struct msgbuf *bufp, cur;
> > + struct timeval boottime;
> > + char timebuf[64];
> >
>
> Why 64?
Well, any recommendation? Is the a _MAX define somewhere?
I always use 64 but if there is something better I'll use it.
>
>
> > char *bp, *ep, *memf, *nextp, *nlistf, *p, *q, *visbp;
> > + const char *timefmt = "%d %b %T";
> > kvm_t *kd;
> > + time_t time;
> > size_t buflen, bufpos;
> > long pri;
> > - int ch, clear;
> > + int ch, clear, timeconv;
> >
>
> timeconv should be a bool.
OK...
>
>
> > bool all;
> >
> > all = false;
> > clear = false;
> > + timeconv = false;
> > (void) setlocale(LC_CTYPE, "");
> > memf = nlistf = NULL;
> > - while ((ch = getopt(argc, argv, "acM:N:")) != -1)
> > + while ((ch = getopt(argc, argv, "actf:M:N:")) != -1)
> > switch(ch) {
> > case 'a':
> > all = true;
> > @@ -95,6 +101,12 @@
> > case 'c':
> > clear = true;
> > break;
> > + case 't':
> > + timeconv = true;
> > + break;
> > + case 'f':
> > + timefmt = optarg;
> > + break;
> > case 'M':
> > memf = optarg;
> > break;
> > @@ -109,6 +121,13 @@
> > if (argc != 0)
> > usage();
> >
> > + if( timeconv ) {
> >
>
> So the spacing doesn't meet style(9). "if (timeconv) {" is the preferred
> spacing.
Yes, that's why I offered to re-format it...
>
>
> > + int mib[6] = { CTL_KERN, KERN_BOOTTIME };
> > + size_t l = sizeof boottime;
> >
>
> sizeof(boottime) is the preferred spelling here.
>
>
> > + if (sysctl(mib, 2, &boottime, &l, 0, 0) < 0)
> > + err(1, "sysctl kern.boottime");
> > + }
> > +
> > if (memf == NULL) {
> > /*
> > * Running kernel. Use sysctl. This gives an unwrapped
> > buffer
> > @@ -200,7 +219,22 @@
> > }
> >
> > (void)strvisx(visbp, p, nextp - p, 0);
> > - (void)printf("%s", visbp);
> > + bufpos = strspn( visbp+1, "0123456789" ) + 1;
> >
>
> No check for overflow here. *bufpos may point to a NUL and +1 will point
> it past that NULL.
bufpos is no pointer but size_t. visbp[ bufpos ] will never point past
'\0' as I start at visbp + 1 for the strspn().
But the test of visbp[ bufpos+1 ] below is not OK. I'll fix that...
> It shouldn't, but it might. And you should do something more like
>
> if (!timeconv) {
> <the existing printf>;
> } else {
> decode the buffer
> }
I din't want to duplicate <the existing printf> too often in case
timeconv is desired but fails later for whatever reasons. So would
is the recommendation here?
1. Whenever it fails, do <the existing printf> and continue?
2. Place <the existing printf> at the end and "goto" it?
3. ??
>
> But you should check things in the right order so you can't overflow. Your
> code already has a fallback for printing things...
>
> *ALSO* The parsing is wrong. When show_timestamp=2 it will have a
> fractional part (so .xxxx) that should be parsed as well. This also throws
> off the math that just adds to the second of time timeval.
Right, the conversion gets aborted in this case (I never used
show_timestamp=2 ;-)). I'll have a look at it...
>
> > + if( timeconv != true
> > + || bufpos < 2
> > + || visbp[0] != '['
> > + || visbp[bufpos] != ']'
> > + || visbp[bufpos+1] != ' ' ) {
> > + (void)printf("%s", visbp);
> > + continue;
> > + }
> > +
> > + visbp[bufpos] = '\0';
> > + time = boottime.tv_sec + strtoull( visbp+1, NULL, 10 );
> > + if( strftime( timebuf, sizeof timebuf, timefmt, localtime(
> > &time ) ) != 0 )
> >
>
> Same spacing. comment about parens. And note that we're strongly
> encouraging {} even for single line statements.
OK...
> ...
> So that's my feedback on the first round of patches. You should do a github
> pull request
> or a Phabricator review. Do not put this in a bugzilla bug. Bugzilla is not
> a code review tool
> and I suspect that feedback on the revision will be needed (if nothing else
> than for the man
> page that we've not seen yet). github is likely easiest for you, and I'll
> keep an eye out there.
> Phabricator is a lot more work to get started. If you are going to do a
> bunch of reviews, it
> may make sense to climb the hill (or for other parts of the code whose
> maintainers don't do
> github). If it's just this one, and you have a github account, I'd
> recommend that as the path
I used Phabricator once (https://reviews.freebsd.org/D46313) but I think
I'll follow your advice and try the github way...
-Andre
