Messages in this thread | | | From | Namhyung Kim <> | Subject | Re: [PATCH 3/3] tools lib traceevent: Introduce pevent_strerror | Date | Fri, 15 Jun 2012 18:04:52 +0900 |
| |
Hi, Steve
On Thu, 14 Jun 2012 23:27:05 -0400, Steven Rostedt wrote: > On Wed, 2012-06-13 at 12:02 +0900, Namhyung Kim wrote: >> 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. >
I'm not sure I understood you. Are you saying the pevent_get_error should look like:
char *pevent_get_error(int errcode, ...)
? So, what's the difference to my original pevent_strerror ?
> 2) Make it TLS, although honestly I've never made dynamic variables > that, and didn't even realize that you could. >
Right, it should be TSD (thread-specific data) for dynamic ones, I guess.
> 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. >
Yeah, I like the #1 too.
> 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); >
Do we really need these two? I think having a single API is just enough, IMHO.
Thanks, Namhyung
| |