Re: [PATCH 1/2] gdb: Allow target description to be dumped even when it is remote

[Pedro Alves <palves@redhat.com>]

On 6/11/20 11:41 AM, Andrew Burgess wrote:
> The maintenance command 'maintenance print c-tdesc' can only print the
> target description if it was loaded from a local file, or if the local
> filename is passed to the maintenance command as an argument.
> 
> Sometimes it would be nice to know what target description GDB was
> given by the remote, however, if I connect to a remote target and try
> this command I see this:
> 
>   (gdb) maintenance print c-tdesc
>   The current target description did not come from an XML file.
>   (gdb)
> 
> Which is not very helpful.
> 
> This commit changes things so that if the description came from the
> remote end then GDB will use the fake filename 'target.xml' as the
> filename for the description, GDB will then create the C description
> of the target as if it was in a file 'target.xml'.
> 
> I originally added this functionality so I could inspect the
> description passed to GDB by the remote target.  After using this for
> a while I realised that actually having GDB recreate the XML would be
> even better, so a later commit will add that functionality too.
> 
> Still, given how small this patch is I thought it might be nice to
> include this in GDB anyway.
> 
> While I was working on this anyway I've added filename command
> completion to this command.
> 
> gdb/ChangeLog:
> 
> 	* target-descriptions.c (maint_print_c_tdesc_cmd): Use fake
> 	filename for target descriptions that came from the target.
> 	(_initialize_target_descriptions): Add filename command completion
> 	for 'maint print c-tdesc'.
> ---
>  gdb/ChangeLog             |  7 +++++++
>  gdb/target-descriptions.c | 14 ++++++++++----
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 20a3a640f4f..55ea416d69a 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -1680,7 +1680,12 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
>      error (_("There is no target description to print."));
>  
>    if (filename == NULL)
> -    error (_("The current target description did not come from an XML file."));
> +    {
> +      printf_unfiltered (_("The current target description was fetched "
> +			   "from the target, using\n'target.xml' as a fake "
> +			   "filename.\n\n"));

That "," makes it read a little bit ambiguously.  Try reading the sentence
without the "," to see what I mean:

  The current target description was fetched from the target using
 'target.xml' as a fake filename.

This can be read as GDB having read the remote fake 'target.xml' filename,
like:
  fetch_available_features_from_target ("target.xml", ops);
which is what it always does anyway...

I'd suggest a hard period (and line break after the period) instead:

  The current target description was fetched from the target.
  Using 'target.xml' as a fake filename.

But, why do we need to provide a fake name at all?  Isn't the only use of
that filename to print it in the comment, here:

 /* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
   Original: target.xml */

How about just doing this:

--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -1680,7 +1680,7 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
     error (_("There is no target description to print."));
 
   if (filename == NULL)
-    error (_("The current target description did not come from an XML file."));
+    filename = "fetched from current target";

with no printf_unfiltered call, and then we get:

 (gdb) maint print c-tdesc 
 /* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
   Original: fetched from current target */

Thanks,
Pedro Alves