Hi Laurent,
On 17/09/2020 02:17, Laurent Pinchart wrote:
> Hi Kieran,
>
> Thank you for the patch.
>
> On Wed, Sep 16, 2020 at 03:43:01PM +0100, Kieran Bingham wrote:
>> Introduce a new utility which prefixes a monotonic timestamp rendered in the
>> same format as the kernel logs to all lines fed in through stdin.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
>> ---
>> src/Makefile | 10 +++++++---
>> src/monotonic-ts.c | 37 +++++++++++++++++++++++++++++++++++++
>> 2 files changed, 44 insertions(+), 3 deletions(-)
>> create mode 100644 src/monotonic-ts.c
>>
>> diff --git a/src/Makefile b/src/Makefile
>> index d7f901f58be6..67216e81ffc4 100644
>> --- a/src/Makefile
>> +++ b/src/Makefile
>> @@ -7,18 +7,22 @@ CFLAGS ?= -O0 -g -W -Wall -Wno-unused-parameter -Iinclude
>> LDFLAGS ?=
>> LIBS := -lm
>> GEN-IMAGE := gen-image
>> +MONOTONIC_TS := monotonic-ts
>
> s/MONOTONIC_TS/MONOTONIC-TS/ to match GEN-IMAGE ?
Hrm, habits of thinking I can't use a hyphen here, yet - clearly we can ;-)
>>
>> %.o : %.c
>> $(CC) $(CFLAGS) -c -o $@ $<
>>
>> -all: $(GEN-IMAGE)
>> +all: $(GEN-IMAGE) $(MONOTONIC_TS)
>>
>> $(GEN-IMAGE): gen-image.o
>> $(CC) $(LDFLAGS) -o $@ $^ $(LIBS)
>>
>> +$(MONOTONIC_TS): monotonic-ts.o
>> + $(CC) $(LDFLAGS) -o $@ $^ $(LIBS)
>> +
>> clean:
>> -rm -f *.o
>> - -rm -f $(GEN-IMAGE)
>> + -rm -f $(GEN-IMAGE) $(MONOTONIC_TS)
>>
>> install:
>> - cp $(GEN-IMAGE) $(INSTALL_DIR)/
>> + cp $(GEN-IMAGE) $(MONOTONIC_TS) $(INSTALL_DIR)/
>
> I'd split this on two lines but I'm not sure why, so feel free to ignore
> this :-)
It was two lines, and I looked at it and thought it should be one.
But two makes it easier to extend without modifying the existing
lines... so I actaully prefer two...
>> diff --git a/src/monotonic-ts.c b/src/monotonic-ts.c
>> new file mode 100644
>> index 000000000000..fcb671e06d27
>> --- /dev/null
>> +++ b/src/monotonic-ts.c
>> @@ -0,0 +1,37 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/* SPDX-FileCopyrightText: 2020 Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx> */
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <time.h>
>> +
>> +int main(int argc, char ** argv)
>> +{
>> + struct timespec tp;
>> + char *line = NULL;
>> + size_t size = 0;
>> + const char *label = "";
>> +
>> + if (argc > 1)
>> + label = argv[1];
>> +
>> + /*
>> + * Explicitly set line buffering on stdin to be sure it is delivered
>> + * in a timely fashion for our timestamping purposes when data is fed
>> + * through a pipe.
>> + */
>> + setlinebuf(stdin);
>> +
>> + do {
>> + if (getline(&line, &size, stdin) <= 0)
>> + break;
>> +
>> + clock_gettime(CLOCK_MONOTONIC, &tp);
>> + printf("[%ld.%.9ld]%s %s", tp.tv_sec, tp.tv_nsec, label, line);
>> + } while (!feof(stdin));
>> +
>> + free(line);
>> +
>> + return 0;
>> +}
>> +
>
> Extra blank line.
It's ok - you can have that one for free ;-)
> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Thanks, will update.
--
Regards
--
Kieran
