Re: [PATCH] libtracefs: Implement tracefs_warning()

[Steven Rostedt <rostedt@xxxxxxxxxxx>]

On Wed,  7 Apr 2021 08:11:54 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@xxxxxxxxx> wrote:

> The warning() function is used in a lot of places in the libracefs
> library, and it is implemented as a dummy weak function. There is a
> weak implementation of the same function in traceevent library, which
> could cause a problem when both are used in an application.
> Replaced warning() with tracefs_warning() and implemented it as a
> weak function, specific to this library.
> Added a __weak define in the library internal header file.

I've been thinking about this more, and I think we only want one function
to override, and that would be:

void __weak tep_vwarning(const char *name, const char *fmt, va_list ap)
{
	if (errno)
		perror(name);
	
	fprintf(stderr, "  ");
	vfprintf(stderr, fmt, ap);
	fprintf(stderr, "\n");
}


And simply have:

void tep_warning(const char *fmt, ...)
{
	va_list ap;

	va_start(ap, fmt);
	tep_vwarning("libtraceevent", fmt, ap);
	va_end(ap)
}

and

void tracefs_warning(const char *fmt, ...)
{
	va_list ap;

	va_start(ap, fmt);
	tep_vwarning("libtracefs", fmt, ap);
	va_end(ap)
}

and

void tracecmd_warning(const char *fmt, ...)
{
	va_list ap;

	va_start(ap, fmt);
	tep_vwarning("libtracecmd", fmt, ap);
	va_end(ap)
}

Then the application only needs to overwrite one function if they want
full control of the output. And it can use the name field to know if it
wants to do something different with different libraries.

Thus, trace-cmd could do:

void tep_vwarning(const char *name, const char *fmt, va_list ap)
{
	if (errno)
		perror("trace-cmd");

	fprintf(stderr, "  ");
	vfprintf(stderr, fmt, ap);
	fprintf(stderr, "\n");
}

and this will make all the warning messages from all the libraries print
the error from "trace-cmd" (we want the tool not the library), just like it
does today.

We could also have kernelshark have:

void tep_vwarning(const char *name, const char *fmt, va_list ap)
{
	char buf[BUFSIZ + 1];
	char *p = &buf[0];
	int r, left = BUFSIZ;

	if (errno) {
		r = snprintf(p, left, "kernelshark: %s\n", strerror(errno));
		left -= r;
		p += r;
	}

	if (left > 0) {
		r = snprintf(p, left, " ");
		p += r;
		left -= r;
	}

	if (left > 0) {
		r = vsnprintf(p, left, fmt, ap);
		left -= r;
		p += r;
	}

	if (left > 0)
		snprintf(p, left, "\n");

	gui_popup(buf);
}

Doing the above with a single function tep_vprintf() means that the
application doesn't need to define multiple functions.

-- Steve