This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]