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"


Hello Doug,

I do not consider it too significant, it is just about comments:


On Tue, 23 Apr 2013 01:55:51 +0200, Doug Evans wrote:
>  > > --- 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,

The difference between existing free_so and added clear_solist may not be
clear as they are similar.  So I wanted to clarify how these two functions
really differ.


> 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.

OK, maybe the last sentence is excessive.


>  > >      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.

Bu there is a difference so just making a copy is incorrect - clear_solib
applies to the whole solib backend (therefore to all its so_list entries).
But clear_solist applies only to single SO entry.


>  >     /* 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.

It was again trying to clarify how this function differs from free_so.
You cannot pass SO back to the backend after free_so is called.


> I can't think of anything useful, I can glean what I need from the code
> in this particular case.  Suggestions?

You should never need to look at (a single one at the moment) implementation
as that may lead you into misleading assumptions about the interface.

The goal is to make the interface clearly defined first.  Implementation(s)
is(are) then dependent on it, not vice versa.


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


Thanks,
Jan


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