Re: [PATCH v3] gdb/hppa: guess g packet size

[Simon Marchi <simark@simark.ca>]

> @@ -2991,15 +3014,32 @@ hppa_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>      = gdbarch_alloc (&info, gdbarch_tdep_up (new hppa_gdbarch_tdep));
>    hppa_gdbarch_tdep *tdep = gdbarch_tdep<hppa_gdbarch_tdep> (gdbarch);
> 
> -  /* Determine from the bfd_arch_info structure if we are dealing with
> -     a 32 or 64 bits architecture.  If the bfd_arch_info is not available,
> -     then default to a 32bit machine.  */
> -  if (info.bfd_arch_info != NULL)
> -    tdep->bytes_per_address =
> -      info.bfd_arch_info->bits_per_address / info.bfd_arch_info->bits_per_byte;
> +  /* Determine from the target description if we are dealing with
> +     a 32 or 64 bits architecture. If the target description is not
> +     available, then check whether bfd_arch_info could be used.
> +     Otherwise default to a 32bit machine.
> +  */
> +  if (info.target_desc != nullptr)
> +    {
> +      if (tdesc_property (info.target_desc, PROPERTY_GP64) != nullptr)
> +	tdep->bytes_per_address = 8;
> +      else if (tdesc_property (info.target_desc, PROPERTY_GP32) != nullptr)
> +	tdep->bytes_per_address = 4;
> +      else
> +	{
> +	  warning (_("target returned a target description this gdb doesn't "
> +		     "support for the HP/PA architecture"));

Good idea to put out a warning.  I would suggesting some minor changes
to the message:

	  warning (_("The target returned a target description but this gdb "
		     "doesn't support target descriptions for the HP/PA
		     "architecture"));

Just thinking out loud: it would actually be nice to have this warning
for all architectures that don't support target descriptions, if there
was a way to put it at a single common place.

Other than that, the patch looks fine.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Do you have push access or would you like me to push the patch on your
behalf (with the modification proposed above)?

Simon