Re: [PATCH 3/3] tools lib traceevent: Introduce pevent_strerror

[Steven Rostedt <rostedt@goodmis.org>]

On Wed, 2012-06-13 at 12:02 +0900, Namhyung Kim wrote:
> Hi,
> 
> On Tue, 12 Jun 2012 14:01:54 -0400, Steven Rostedt wrote:
> > On Tue, 2012-06-12 at 16:42 +0900, Namhyung Kim wrote:
> >> +int pevent_strerror(struct pevent *pevent, enum pevent_errno errnum,
> >> +		    char *buf, size_t buflen)
> >
> > Hmm, actually I wonder if we should put the error into the pevent
> > structure. Then we wouldn't even need to waste time to pass the data
> > through.
> >
> > That is, you can simply do:
> >
> > 	ret = pevent_foo();
> > 	if (ret < 0) {
> > 		pevent_strerr(pevent, buf, buflen);
> > 		printf("%s\n", buf);
> > 	}
> >
> > Perhaps even include a pevent_perror(), to just do:
> >
> > 	if (ret < 0) {
> > 		pevent_perror(pevent);
> > 		return ret;
> > 	}
> >
> 
> I thought something like this, but worried about the thread-safety. What
> about if more than one thread call pevent functions for a same pevent
> concurrently?  Should we make the pevent->errno TLS?

I hate threads :-)

Anyway, there's a couple of things that can be done:

1) have two functions. A simple 'pevent_stderr()' that does the above,
and perhaps a pevent_get_error() that can be passed the return value of
a previous command, and give you the error string for it. This is thread
safe for programs that require it. The ret is always returned.

2) Make it TLS, although honestly I've never made dynamic variables
that, and didn't even realize that you could.

I'm liking the explicit 'make this thread safe' method (#1). As it may
be for a gui, a separate thread may be used to print out the error
messages, and making it thread unique will make that difficult.

If you need the code to be thread safe, have all errors do:

	ret = pevent_foo();
	if (ret < 0) {
		pevent_strerr_val(ret, buf, buflen);


For programs that do not need to be thread safe, then:

	ret = pevent_foo();
	if (ret < 0) {
		pevent_strerr(pevent, buf, buflen);


-- Steve