[Toybox] [PATCH] fixes for the 'ts' command

[Rob Landley]

On 9/12/23 20:25, Oliver Webb via Toybox wrote:
> This patch fixes a memory leak in ts by making sure it frees the pointers given by xgetline()

Blah, conventional getline() does a realloc() recycling the same buffer,
xgetline returns a string to be freed. I knew that. :P

Local variable declarations go at the start of blocks, documented coding style
thing for the project:

  https://landley.net/toybox/design.html#:~:text=Variable%20declarations

You can put them at the start of existing { local blocks } if you prefer, but it
doesn't help with modern optimizers doing liveness analysis, and I personally
tend to group them up top where I can see them.

> I knew about this leak for a few days and was planning to submit a patch for it some time in the future, 
> but now that ts got promoted I feel like I should submit it sooner rather then later
> 
> The other fix is that if it's doing -i or -s it uses gmtime() instead of localtime() 

rel = !!(toys.optflags&~FLAG_m) is a fancy way of saying (FLAG(i) || FLAG(s))
but now that I look at it I should just change it to be the explicit version and
let the optimizer sort it out. (Last I checked it doesn't even when I use |
instead of || but eh, more readable beats saving a couple bytes...)

Anyway: the "is either of these two flags set" test is already cached in a local
"relative time" variable, I'll fix it up...

> so the timestamp starts at 00:00:00 instead of something like 20:00:00 for PDT or 18:00:00 for CST.

Sigh, and thus the subtraction does NOT zero it out. (Elliott can confirm how
tired all the timezone plumbing makes me.)

Also, I have no idea how to test this command. I've caught the busybox ts -i
producing "0" offsets with ts -i and sleep 1 in the loop, because a 2ms delay
between the left side of the pipe emitting the string and the right side of the
pipe getting scheduled to process the string means the _next_ string seems to
show up 998 milliseconds later, which rounds down. Which is why I wanted -m in
there, but that shows _more_ jitter and thus a less reproducible test. (Unless I
do $((math)) on the result, but... what does that prove? What am I _testing_?
Interpreting the results on a loaded system is an AI-complete problem, unless
I'm just testing "number of digits emitted and where the punctuation goes"?)

Applied, with my two tweaks on top of it in a second commit. No regression tests
to run, but:

$ for i in one two three four five; do echo $i; sleep 1; done | ./ts -im
00:00:00.000 one
00:00:01.004 two
00:00:01.001 three
00:00:01.001 four
00:00:01.002 five

Looks ok to me I guess?

Thanks,

Rob