Re: [RFA] Handle solaris dynamic linker name change.

[Joel Brobecker <brobecker at adacore dot com>]

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

> +    /* 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.  We can discuss my question about /usr/lib and /lib
as a separate patch (this is already an improvement that we don't
need to delay).

Thanks!
-- 
Joel