This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
Re: [patch v7 3/5] x86* unwinder: libdwfl/
- From: Mark Wielaard <mjw at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Wed, 23 Oct 2013 14:18:10 +0200
- Subject: Re: [patch v7 3/5] x86* unwinder: libdwfl/
On Tue, 2013-10-22 at 22:32 +0200, Jan Kratochvil wrote:
> On Mon, 21 Oct 2013 21:19:40 +0200, Jan Kratochvil wrote:
> > On Sat, 19 Oct 2013 03:05:23 +0200, Mark Wielaard wrote:
> > > Again mostly nitpicks. The only real issue I would like you to
> > > think/comment on is around the dwfl_thread_next comments.
> >
> > Maybe you want threads to be iterated through callbacks?
>
> Implemented it in the attached diff and checked into jankratochvil/unwindx86.
I like it. It seems to simplify the code.
> @@ -260,47 +246,50 @@ dwfl_frame_thread (Dwfl_Frame *state)
> }
> INTDEF(dwfl_frame_thread)
>
> -Dwfl_Thread *
> -dwfl_next_thread (Dwfl *dwfl, Dwfl_Thread *prev_thread)
> +int
> +dwfl_getthreads (Dwfl *dwfl, int (*callback) (Dwfl_Thread *thread, void *arg),
> + void *arg)
> [...]
> + nthread->tid = process->callbacks->next_thread (dwfl, nthread,
> + process->callbacks_arg,
> + &nthread->callbacks_arg);
> + if (nthread->tid < 0)
> + {
> + Dwfl_Error saved_errno = dwfl_errno ();
> + thread_free (nthread);
> + __libdwfl_seterrno (saved_errno);
> + return -1;
> + }
thread_free doesn't set dwfl_errno, so should be save to call without
saving/restoring it.
I think you can now also simplify the Dwfl_Thread_Callbacks next_thread.
It doesn't really seem to need the Dwfl_Thread that is being constructed
since it doesn't seem contain anything that helps with constructing the
next one. So I would change it to something like:
/* Called to iterate through threads. Returns next TID (thread ID) on
success, a negative number on failure and zero if there are no more
threads. dwfl_errno () should be set if negative number has been
returned. *THREAD_ARGP will be NULL on the first call and may be
optionally set by the implementation. On next calls **THREAD_ARGP
will point to the *THREAD_ARGP value set last time. THREAD_ARGP is
never NULL. *THREAD_ARGP will be passed to set_initial_registers
or thread_detach callbacks together with Dwfl_Thread *thread.
This method must not be NULL. */
pid_t (*next_thread) (Dwfl *dwfl, void *dwfl_arg, void **thread_argp)
__nonnull_attribute__ (1, 2);
That way the callback can keep any state it wants between next_thread
calls through *THREAD_ARGP if it wants.
Does that make sense?
Cheers,
Mark