[PATCH] libdwfl: Add dwfl_getthread and dwfl_getthread_frames.
Mark Wielaard
mjw at redhat.com
Mon Dec 23 21:58:28 UTC 2013
On Mon, 2013-12-23 at 18:53 +0100, Jan Kratochvil wrote:
> > Updated patch attached.
>
> Fine with that with the comments below.
Pushed to master with the comments fixed.
> > +static int
> > +get_one_thread_cb (Dwfl_Thread *thread, void *arg)
> > +{
> > + struct one_arg *oa = (struct one_arg *) arg;
> > + if (! oa->seen && INTUSE(dwfl_thread_tid) (thread) == oa->tid)
> > + {
> > + oa->seen = true;
> > + return oa->callback (thread, oa->arg);
>
> Maybe it was intentionall to keep the code simple - but if oa->callback
> returns DWARF_CB_OK then it will needlessly continue iterating the threads
> despite it can never find that TID again.
Yes I missed that. Added a ret argument to struct one_arg to catch to
actual callback return value and return DWARF_CB_ABORT getthread then
returns that value when it sees DWARF_CB_ABORT with seen == true.
> > +/* Note not currently exported, will be when there are more Dwfl_Thread
> > + properties to query. Use dwfl_getthread_frames for now directly. */
> > +static int
> > +dwfl_getthread (Dwfl *dwfl, pid_t tid,
>
> It is no longer public so I think it should not have dwfl_* prefix.
Removed dwfl_ prefix.
> > + int (*callback) (Dwfl_Thread *thread, void *arg),
> > + void *arg)
> > +{
> > + Dwfl_Process *process = dwfl->process;
> > + if (process == NULL)
>
> As the function is no longer public this can be rather an assert.
It could, but since we do want to make this public at one point I kept
it as is, so we don't forget the actual logic needed.
> > + struct one_arg oa = { .tid = tid, .callback = callback,
> > + .arg = arg, .seen = false };
> > + int err = INTUSE(dwfl_getthreads) (dwfl, get_one_thread_cb, &oa);
> > + if (err == DWARF_CB_OK && ! oa.seen)
>
> __libdwfl_seterrno is not called.
Set to DWFL_E_ERRNO with errno = ESRCH.
Added the same to pid_getthread which also failed to set the error.
> > + /* Called to get a specific thread. Returns true if there is a
> > + thread with the given thread id number, returns false if no such
> > + thread exists and may set dwfl_errno in that case. THREAD_ARGP
>
> s/may/it will/. Setting errno has to be mandatory, not voluntary.
Changed.
Thanks,
Mark
More information about the elfutils-devel
mailing list