PelionIoT / mbed-trace

mbed trace library
Apache License 2.0
18 stars 15 forks source link

Introduce stream support #82

Open jupe opened 6 years ago

jupe commented 6 years ago

Status

IN PROGRESS

Migrations

NO

Description

This allows to pipe traces to any standard FILE* stream. NOTE: This should not break existing API's or behaviours.

This allows to do this:

FILE *fh = fopen("./debug.log", "a");
mbed_trace_set_pipe(fh);
tr_debug("debugging to file");

Todo

jupe commented 6 years ago

This PR changes default "printer" function from puts to fputs, which behaviours slightly differently (eg buffers/flushing point of view). When using custom printer there is wrapper function which provides fputs -kind of API and calls that custom printer which are backward compatible. Haven't analyse more deeply does this affects to performance and how much if. Opinions about this ?

kjbracey commented 6 years ago

Feels a little bit unnecessary.

I've always been quite happy to provide a print function that does fputs(line, X); fputc('\n', X);. Doesn't seem like this is really easier.

I guess a general config option to include a newline in the output call would streamline it a bit.

kjbracey commented 6 years ago

BTW, puts shouldn't be different from fputs(stdout), except for the fact it includes a newline.

In POSIX puts being a single call means it's effectively equivalent to

flockfile(stdout);
fputs(line, stdout);
fputc('\n', stdout);
funlockfile(stdout);

On non-POSIX systems, the atomicity of a single library call isn't guaranteed, and we're relying on our mutex anyway.

But having an option to make the formatting core stick on a \n along with all the other headers/trailers makes perfect sense to streamline this generally - whether or not the friendly "stream" thing is added.

jupe commented 6 years ago

Feels a little bit unnecessary. I've always been quite happy to provide a print function that does fputs(X, line); fputc('\n', X);. Doesn't seem like this is really easier.

True, basically same thing can be achieved already with a bit more code. Idea was to "simplify" output a bit so that output is always standard FILE stream so that output can be something else than stdout easily.. Do you see some (other negative) side-effects with this PR ?

I guess a general config option to include a newline in the output call would streamline it a bit.

I think about it also. Would be easy task.

In POSIX puts being a single call means it's effectively equivalent to flockfile(stdout);

Maybe our mutex API calls could have stream argument so it could be flockfile by default in POSIX..

kjbracey commented 6 years ago

I've always thought part of the concept was that there was no assumption anywhere that the output actually be a stream - it could be an insertion into a circular buffer, or transmission as a network packet.

Hence the entry being delivered as a simple single "unit".

I'd kind of like that purity preserved in the core, but I guess a helper that does set_stream_output(FILE *) makes sense. But it can just be equivalent to

remember_stream
set_output(internal_helper_fputs_function)
turn on line endings
kjbracey commented 6 years ago

Locking is a slightly fiddly concept - not sure flockfile fits directly. If we were to attempt an extra buffering layer for buffering from IRQ (as per the universal trace), you'd need a new locking concept anyway, which I've been pondering.

You'd need one lock to protect the formatting and temp buffers in the tr_xxx calls, and a separate lock to protect interaction between buffer drain to screen and other screen users like CLI. That second lock could actually be not visible to the formatter, and built into the output function. But at the minute the CLI assumes it's all one lock, and uses the formatter lock to avoid output clash.