Re: [PATCH v3] Support glibc multiple namespace extension

[Kevin Buettner via Gdb-patches <gdb-patches@sourceware.org>]

Hi H.J.,

This work looks pretty good to me.  I found a couple of coding style nits
and have a question/request; see below.

On Sat, 2 Oct 2021 13:43:40 -0700
"H.J. Lu via Gdb-patches" <gdb-patches@sourceware.org> wrote:

[...]
> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
> index 3de1bb9c7f7..9d851ba8930 100644
> --- a/gdb/solib-svr4.c
> +++ b/gdb/solib-svr4.c
[...]
> @@ -1335,6 +1366,18 @@ svr4_current_sos_direct (struct svr4_info *info)
>    if (lm)
>      svr4_read_so_list (info, lm, 0, &link_ptr, 0);
>  
> +  /* Get the next namespace from the r_next field.  */
> +  lm = solib_svr4_r_next (info->debug_base);
> +  while (lm)

Due to the GDB coding standard, this test needs to be turned into an
explicit comparison against 0.  See:

https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Comparison_With_nullptr_And_Zero

(FWIW, I don't really like this part of our standard; I personally
find it more readable to code it the way that you did.  Also, I do
realize that there are existing implicit tests against 0 in this file
and elsewhere and that they do not conform to GDB's current coding
standard.  I'm in no hurry to fix them - but, obviously, I won't
object if someone does.)

> +    {
> +      /* Get the link map in this namespace.  */
> +      CORE_ADDR link_map = solib_svr4_r_map (lm);
> +      if (link_map)

Likewise, for the check above.

> +	svr4_read_so_list (info, link_map, 0, &link_ptr, 0);
> +      /* Go to the next namespace.  */
> +      lm = solib_svr4_r_next (lm);
> +    }
> +
>    cleanup.release ();
>  
>    if (head == NULL)
> @@ -1706,7 +1749,8 @@ solist_update_full (struct svr4_info *info)
>     failure.  */
>  
>  static int
> -solist_update_incremental (struct svr4_info *info, CORE_ADDR lm)
> +solist_update_incremental (struct svr4_info *info, CORE_ADDR debug_base,
> +			   CORE_ADDR lm)
>  {
>    struct so_list *tail;
>    CORE_ADDR prev_lm;
> @@ -1727,8 +1771,15 @@ solist_update_incremental (struct svr4_info *info, CORE_ADDR lm)
>    for (tail = info->solib_list; tail->next != NULL; tail = tail->next)
>      /* Nothing.  */;
>  
> -  lm_info_svr4 *li = (lm_info_svr4 *) tail->lm_info;
> -  prev_lm = li->lm_addr;
> +  /* Don't check shared libraries in other namespaces when updating
> +     shared libraries in a new namespace.  */

Shared libraries in other namespaces aren't being neglected though,
right?

Assuming that's true, could you add a sentence or two to your comment
addressing this concern?

Kevin