On Friday 04 January 2008 18:30:20 Joel Brobecker wrote:
> > > I'd like the feedback from other maintainers about this.
> > 
> > I think it's fine with that improvement or without it; I really doubt
> > the code will trigger anywhere else.  My two cents.
> 
> OK, in that case, let's leave it the way Volodya original proposed it.
> I'd just like us to add a comment in svr4_same to say that we don't
> restrict the check to solaris, but that the chances of facing this
> situation in any other OS are very small.
> 
> Looking again at the patch:
> 
> > 	Ignore change in name of dynamic linker during
> > 	execution.  This also unbreaks pending breakpoints.
> > 	* solist.h (struct target_so_ops): New field
> > 	same.
> > 	* solib-svr4.c (svr4_same): New.
> > 	(_initialize_svr4_solib): Register svr4_same.
> > 	* solib.c (update_solib_list): Use ops->same,
> > 	if available.
> 
> Just a very very minor comment: Your fill-column looks very small, and
> makes it harder to read your sentences. It's supposed to be 74. Don't
> go out of your way to fix, though, in the grand scheme of things,
> I don't think that you're breaking a GNU Coding standard rule (I just
> did a quick check of the section detailing ChangeLogs).
> 
> > +  /* On Solaris, when starting inferior we think that
> > +     dynamic linker is /usr/lib/ld.so.1, but later on,
> > +     the table of loaded shared libraries contains
> > +     /lib/ld.so.1.
> > +     Sometimes one file is a link to another, but sometimes
> > +     they have identical content, but are not linked to each
> > +     other.  */
> 
> Same small fill-column ;-). Actually, the real comment is to remind
> you that an extra comment as explained at the beginning of this email
> would be appreciated.
> 
> > +  if (strcmp (gdb->so_original_name, "/usr/lib/ld.so.1") == 0
> > +      && strcmp (inferior->so_original_name, "/lib/ld.so.1") == 0)
> > +    return 1;
> 
> Is it always going to be in that order (original in /usr/lib and
> gdb in /usr)? Or was this from experience?

This is from experience. Since we don't know why Solaris behave this way,
we probably cannot guess if the other way round is possible, and how
to handle it.

> 
> > +    /* Given two so_list, first from GDB thread list and another
> > +       present in the list returned by current_sos, return 1 if
> > +       they are equal -- referring to the same library.  */
> > +    int (*same) (struct so_list *gdb, struct so_list *inferior);
> 
> I am not a native English speaker, but the above sounds funny.
> I suggest a slightly different version:
> 
> 
>     /* Given two so_list objects, one from the GDB thread list
>        and another from the list returned by current_sos, return 1
>        if they represent the same library.  */
> 
> This is all I have - your patch is pre-approved with the comment
> adjustments.  

Thanks. I've adjusted comments as shown in attachment and checked in.

- Volodya