Re: [PATCH] Abi selection based on set arch command in case of remote debugging for MIPS

[Simon Marchi <simark at simark dot ca>]

Maciej,

Just wondering, would this patch still fall under your maintainership, and do
you plan on reviewing it?  I think it's a followup this, which should give more
context:

  https://sourceware.org/ml/gdb-patches/2018-10/msg00342.html

Denis: In the mean time, I noted some comments about formatting.  Also, you would
need to provide a ChangeLog entry with the change.  See this for more details:

https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_Formatted_GNU_ChangeLog

For this change, it could look like:

	* mips-tdep.c (mips_gdbarch_init): Select ABI based on architecture if no
	binary is provided.

On 2018-10-29 4:18 a.m., Denis Dmitriev wrote:
> Abi selection based on the execution of the "set arch" command in the case
> of remote debugging. Mips. No need to manually configure abi.
> 
> diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
> index 5e0a606..dc6b6181 100644
> --- a/gdb/mips-tdep.c
> +++ b/gdb/mips-tdep.c
> @@ -8085,7 +8085,20 @@ mips_gdbarch_init (struct gdbarch_info info, struct
> gdbarch_list *arches)
>    if (info.abfd && bfd_get_flavour (info.abfd) == bfd_target_elf_flavour)
>      elf_flags = elf_elfheader (info.abfd)->e_flags;
>    else if (arches != NULL)
> +  {

Applicable to all if/else, the braces should indented:

  if (...)
    {
      ...
    }
  else
    {
      ...
    }

>      elf_flags = gdbarch_tdep (arches->gdbarch)->elf_flags;
> +    if (elf_flags == 0)
> +    {
> +      if (info.bfd_arch_info && info.bfd_arch_info->bits_per_address == 64)

When checking if a pointer is null, use the explicit

    if (info.bfd_arch_info != NULL && ...)

or

    if (info.bfd_arch_info != nullptr && ...)

I know there are violations of this rule in some places, but for new code we try
to enforce the GNU style.

> +      {
> +        elf_flags = E_MIPS_ABI_EABI64;
> +      }
> +      else if (info.bfd_arch_info->bits_per_address == 32)
> +      {
> +        elf_flags = MIPS_ABI_EABI32;
> +      }

When there is a single statement, drop the braces:

  if (...)
    elf_flags = E_MIPS_ABI_EABI64;
  else
    elf_flags = MIPS_ABI_EABI32;

> +    }
> +  }
>    else
>      elf_flags = 0;
>    if (gdbarch_debug)
> 
> 

Simon