Hi Rishabh,
sorry for delay
On Fri, Nov 10, 2023 at 03:26:12PM -0800, Rishabh Thukral wrote:
> This change updates the --time-format option in dmesg to enable the
> user to specify it multiple times with different formats with each
good idea
> input providing a timestamp format among the currently supported
> choices. This enables seeing the dmesg logs in both seconds since boot
> and human readable format simultaneously in each row of the log output.
> The sequence of timestamp format is fixed and independent of the order
> in which the user provides the desired formats.
and would be better to support arbitrary (user defined) order?
ctl->time_fmts[ ctl->ntime_fmts++ ] = DMESG_TIMEFTM_FOO;
ctl->time_fmts[ ctl->ntime_fmts++ ] = DMESG_TIMEFTM_BAR;
and print it by a simple loop
for (i = 0; i < ctl->ntime_fmts; i++)
switch (ctl->time_fmts[ i ]) {
...
}
I guess the implementation will be more simple.
> - if (ctl->time_fmt != DMESG_TIMEFTM_RELTIME) {
> + if (ctl->time_fmts[DMESG_TIMEFTM_RELTIME] == TIMESTAMP_FMT_UNSET) {
This code pattern is used pretty often in your patch, maybe add macros
#define is_timefmt_set(c, f) ((c)->time_fmts[DMESG_TIMEFTM_ ##f] == TIMESTAMP_FMT_SET))
#define is_timefmt_unset(c, f) ((c)->time_fmts[DMESG_TIMEFTM_ ##f] == TIMESTAMP_FMT_UNSET))
#define is_timefmt_initset(c, f) ((c)->time_fmts[DMESG_TIMEFTM_ ##f] == TIMESTAMP_FMT_INIT_SET))
and use
if (is_timefmt_set(ctl, RELTIME))
in code.
> @@ -1388,9 +1416,11 @@ int main(int argc, char *argv[])
> .action = SYSLOG_ACTION_READ_ALL,
> .method = DMESG_METHOD_KMSG,
> .kmsg = -1,
> - .time_fmt = DMESG_TIMEFTM_TIME,
I guess you can use
.time_fmts[DMESG_TIMEFTM_TIME] = TIMESTAMP_FMT_INIT_SET,
> .indent = 0,
> };
> + memset(ctl.time_fmts, 0,
> + TOTAL_DMESG_TIMESTAMP_FORMATS_SUPPORTED * sizeof(*(ctl.time_fmts)));
You do not need memset() here. All unspecified struct fields in 'ctl'
are set zero according to C standards (if any other fields are
explicitly initialized).
> + ctl.time_fmts[DMESG_TIMEFTM_TIME] = TIMESTAMP_FMT_INIT_SET;
> int colormode = UL_COLORMODE_UNDEF;
> enum {
> OPT_TIME_FORMAT = CHAR_MAX + 1,
> @@ -1475,7 +1505,9 @@ int main(int argc, char *argv[])
> ctl.action = SYSLOG_ACTION_CONSOLE_ON;
> break;
> case 'e':
> - ctl.time_fmt = DMESG_TIMEFTM_RELTIME;
> + if (ctl.time_fmts[DMESG_TIMEFTM_TIME] == TIMESTAMP_FMT_INIT_SET)
> + ctl.time_fmts[DMESG_TIMEFTM_TIME] = TIMESTAMP_FMT_UNSET;
> + ctl.time_fmts[DMESG_TIMEFTM_RELTIME] = TIMESTAMP_FMT_SET;
This complicated setup will be unnecessary if we will not need fixed
order, right? :-)
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
http://karelzak.blogspot.com
