This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Variable objects in multi-threaded programs
> > > > +static void
> > > > +check_scope (struct varobj *var, int* scope)
> > > > +{
> > >
> > > This function needs a comment. 'check_scope', in
> > > itself, can do just about anything.
> >
> > This is a small function and I think it speaks for itself.
>
> Not in my opinion. 'check_scope' can mean anything.
The function name doesn't speak for itself, but being only about twelve lines
of code, it is easy to see what it does..
> > I don't
> > know if GNU Coding standards dictate how much commenting there should be but
> > I think that in varobj.c there is too much and it makes the code harder to
> > read.
>
> I disagree -- practice shows that the exact semantics of varobj
> logic is still not 100% clear.
But it looks like the coding standards allow us to choose our own style.
>...
> > > > + saved_frame_id = get_frame_id (get_selected_frame (NULL));
> > > > + old_cleanups = make_cleanup_restore_current_thread (inferior_ptid,
> > > > saved_frame_id);
> > >
> > > Presumably, we can move this to the top-level of c_value_of_root, and then
> > > get rid of save/restore of selected frame in varobj_update?
> >
> > No. This is only necessary in the multi-threaded case.
>
> make_cleanup_restore_current_thread arranges for two things to
> be restored:
> - current thread
> - selected frame
>
> We now always save/restore selected frame in varobj_update. And
> changing/saving/restoring current thread is extremely fast. So,
> why not just save both thread and frame in all cases, and simplify
> the logic.
Then we presumably could remove the calls after value_of_root:
/* Restore selected frame. */
fi = frame_find_by_id (old_fid);
if (fi)
select_frame (fi);
but I have no idea how fast do_restore_current_thread_cleanup is. Indirectly,
it calls functions like reinit_frame_cache.
--
Nick http://www.inet.net.nz/~nickrob