This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Fix handling of "set sysroot foo"
- From: Doug Evans <dje at google dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Mon, 22 Apr 2013 16:55:51 -0700
- Subject: Re: [RFA] Fix handling of "set sysroot foo"
- References: <yjt2ppxzsk97 dot fsf at ruffy2 dot mtv dot corp dot google dot com> <20130421075444 dot GA4967 at host2 dot jankratochvil dot net>
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);