Re: [RFA] win32-nat.c: Add dll names if debugevents is on

[Pedro Alves <pedro@codesourcery.com>]

A Friday 13 June 2008 23:00:40, Pierre Muller wrote:
> Thanks for the comments

> >While you're at it, would it be useful to also include the dll
> >load base in the output?
>
>   It’s a good idea, that I implemented easily, see below.
> I choose to use the solib struct to be closer to 'info dll' output.
> The only caveat is that it doesn't match the values given by 'info dll'
> later
> because of the 0x1000 offset added to the start of each Dll,
> should I add  this 0x1000?

I've no strong opinion, handle_unload_dll already erros without ...

>   It seems like so_original_name is never empty,
> because if the dll name is not found, the dll is not recorded at all.
>   And I finally realized that my code was wrong because
> so_original_name and so_name are char array, not pointers...
> I nevertheless check both so_name and so_original_name
> in that order, without knowing if one can be set and
> not the other...

See win32_make_so.  so_name is always built from so_original_name.

>
> Here is an updated patch:
>

This patch surely doesn't work.

> @@ -771,6 +774,18 @@ handle_unload_dll (void *dummy)
>         so->next = sodel->next;
>         if (!so->next)
>           solib_end = so;
> +       if (sodel->so_name != "\0")
> +         {
> +           DEBUG_EVENTS (("gdb: Unloading dll \"%s\".\n",
> +                          (char *) &(sodel->so_name)));
> +         }
> +       else if (sodel->so_original_name != "\0")
> +         {
> +           DEBUG_EVENTS (("gdb: Unloading dll \"%s\".\n",
> +                          (char *) &(sodel->so_original_name)));
> +         }
> +       else
> +         DEBUG_EVENTS (("gdb: Unloading <unknown name> dll.\n"));

<side note>

  Beware that DEBUG_EVENTS is not safe to reguarding dangling else's...

  #define DEBUG_EVENTS(x)	if (debug_events)	printf_unfiltered x

  That should be fixed to something like:

  #define DEBUG_EVENTS(x) \
   do { if (debug_events) printf_unfiltered x ; } while (0)

  Or better, to output to fprintf_unfiltered (gdb_stdlog, ...

  Otherwise, extra braces are indeed required.

</side note>

> +       if (sodel->so_name != "\0")

Anyway, these comparision aren't doing what you think they are.
You are still comparing addresses.  To check for empty string, use
something like:

     if (*sodel->so_name == '\0')


> +           DEBUG_EVENTS (("gdb: Unloading dll \"%s\".\n",
> +                          (char *) &(sodel->so_name)));

And should print a char array like you were doing before:
 DEBUG_EVENTS (("gdb: Unloading dll \"%s\".\n", sodel->so_name));

But, if so_name is built from so_original_name, and you're printing
just so_name in the load event, why extra that churn in the unload event?  
Just:

+         DEBUG_EVENTS (("gdb: Unloading dll \"%s\".\n",
+                        sodel->so_name));

Would be enough, I guess.

-- 
Pedro Alves