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