Re: [PATCH 1/2] [gdb/symtab] Fix style issues in v9 .gdb_index section support

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

Hello Tom,

Tom de Vries <tdevries@suse.de> writes:

> @@ -1217,7 +1217,7 @@ write_shortcuts_table (cooked_index *table, data_buf& shortcuts,
>  	}
>      }
>  
> -  shortcuts.append_uint (4, BFD_ENDIAN_LITTLE, dw_lang);
> +  shortcuts.append_offset (dw_lang);

This is a functional change. Is it intentional? If so, it'd be better as
a separate patch.

>    shortcuts.append_offset (main_name_offset);
>  }
>  
> diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
> index b96eaa96e23..34c5dfd5803 100644
> --- a/gdb/dwarf2/read-gdb-index.c
> +++ b/gdb/dwarf2/read-gdb-index.c
> @@ -777,10 +777,10 @@ static void
>  set_main_name_from_gdb_index (dwarf2_per_objfile *per_objfile,
>  			      mapped_gdb_index *index)
>  {
> -  const auto expected_size = 4 + sizeof (offset_type);
> +  const auto expected_size = 2 * sizeof (offset_type);

This is also a functional change.

>    if (index->shortcut_table.size () < expected_size)
>      /* The data in the section is not present, is corrupted or is in a version
> -     * we don't know about. Regardless, we can't make use of it. */
> +       we don't know about. Regardless, we can't make use of it. */

One further style issue that could be fixed here is the lack of double
space after period.

>      return;
>  
>    auto ptr = index->shortcut_table.data ();
> @@ -794,7 +794,7 @@ set_main_name_from_gdb_index (dwarf2_per_objfile *per_objfile,
>    if (dw_lang == 0)
>      {
>        /* Don't bother if the language for the main symbol was not known or if
> -       * there was no main symbol at all when the index was built. */
> +	 there was no main symbol at all when the index was built. */
>        return;

Also here, double space after period is lacking.

>      }
>    ptr += 4;

> diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
> index 1d9432c5c11..c92474d8b9d 100644
> --- a/gdb/dwarf2/read.h
> +++ b/gdb/dwarf2/read.h
> @@ -246,11 +246,11 @@ struct dwarf2_per_cu_data
>    std::vector <dwarf2_per_cu_data *> *imported_symtabs = nullptr;
>  
>    /* The original DW_LANG_* value of the CU, as provided to us by
> -   * DW_AT_language. It is interesting to keep this value around in cases where
> -   * we can't use the values from the language enum, as the mapping to them is
> -   * lossy, and, while that is usually fine, things like the index have an
> -   * understandable bias towards not exposing internal GDB structures to the
> -   * outside world, and so prefer to use DWARF constants in their stead. */
> +     DW_AT_language. It is interesting to keep this value around in cases where
> +     we can't use the values from the language enum, as the mapping to them is
> +     lossy, and, while that is usually fine, things like the index have an
> +     understandable bias towards not exposing internal GDB structures to the
> +     outside world, and so prefer to use DWARF constants in their stead. */

I think the lack of double space after period here are fixed in the
other patch you sent.

-- 
Thiago