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. > --- 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? > 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. > 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. > + 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. It shouldn't, but it might. And you should do something more like if (!timeconv) { <the existing printf>; } else { decode the buffer } 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. > + 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. > + (void)printf( "[%s]%s", timebuf, visbp + bufpos + > 1 ); > + else > + (void)printf("%s", visbp); > } > exit(0); > } > @@ -208,6 +242,6 @@ > void > usage(void) > { > - fprintf(stderr, "usage: dmesg [-ac] [-M core [-N system]]\n"); > + fprintf(stderr, "usage: dmesg [-ac] [-t [-f output_fmt]] [-M core > [-N system]]\n"); > exit(1); > } > 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 of least resistance. github also has the advantage that it will give a mini-review on style as well as making sure that the authorship is right. Warner > > -Andre > >
