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: [RFA] Fix handling of "set sysroot foo"


Jan Kratochvil writes:
 > On Fri, 12 Apr 2013 19:59:16 +0200, Doug Evans wrote:
 > > I kinda like and dislike clear_solist as the new name for free_so_symbols.
 > > - the old free_so_symbols does more than just free
 > > - target_so_ops.clear_solib is used for similar "reset and/or free" purposes
 > > - it's nice to have the target function's name match
 > > - target_so_ops function free_so already exists
 > > Hence the rename.
 > 
 > I would find more appropriate clear_so (for the function, the virtual method
 > declaration and its svr4 implementation).
 >  * clear_solib is for the whole backend, not just for one struct so_list.
 >  * free_so is for one struct so_list.
 > 
 > (There remains the problem "struct so_list" is used for both the entry and the
 > whole list as it contains "next".)

I can go along with this.

 > Also svr4_free_so could now have:
 > 	gdb_assert (!so->lm_info->l_addr_p);
 > 
 > 
 > > --- solist.h	1 Jan 2013 06:32:51 -0000	1.38
 > > +++ solist.h	12 Apr 2013 17:56:09 -0000
 > > @@ -88,6 +88,10 @@ struct target_so_ops
 >        /* Free the link map info and any other private data structures
 > >         associated with a so_list entry.  */
 > 
 > +         clear_so method is always called before free_so is called.
 > +         SO is then never passed to the backend again.

This is a comment on an existing function, not something I've added,
and I'm not sure what the point of the second sentence is, really.
free_so frees the object - never passing it to *anything* again is kinda implied.

 > 
 > >      void (*free_so) (struct so_list *so);
 > >  
 > > +    /* Reset or free private data structures associated with
 > > +       so_list entries.  */
 > 
 > Singular form.

The style is just copying/tweaking of existing text (clear_solib).  C'mon dude.

 >     /* Reset or free private data structures associated with
 >        the SO entry.  */
 > +      SO then can be passed to the backend for futher use.

If we're going to add more here, I think we need something different.
That text doesn't tell me anything useful.
I can't think of anything useful, I can glean what I need from the code
in this particular case.  Suggestions?

 > > +    void (*clear_solist) (struct so_list *so);
 > > +
 > >      /* Reset or free private data structures not associated with
 > >         so_list entries.  */
 > >      void (*clear_solib) (void);


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