Re: [PATCH, v3] Add code for processing version 5 DWP files (for use with DWARF v5)

[Tom Tromey <tom@tromey.com>]

>>>>> "Caroline" == Caroline Tice via Gdb-patches <gdb-patches@sourceware.org> writes:

Caroline> The changes to include/dwarf2.h were accepted by binutils and have
Caroline> gone in, so I've removed those changes from this patch.

Thank you for the patch.

I read it and I have a few minor nits, plus one actual bug.

Caroline> -  if (version == 2)
Caroline> +  if ((version == 2) || (version == 5))

Please remove the extra parentheses on this line.

Caroline> +  else // version == 5

gdb still only uses /* */-style comments.

Caroline> +      int i;

This should be local to the for loop:

Caroline> +      for (i = 0; i < nr_columns; ++i)

... for (int i = ...)

Caroline> +      htab->section_pool.v5.sizes =
Caroline> +	htab->section_pool.v5.offsets + (sizeof (uint32_t)

The '=' should go at the start of the second line here.

Caroline> +static struct dwo_unit *
Caroline> +create_dwo_unit_in_dwp_v5 (struct dwarf2_per_objfile *dwarf2_per_objfile,
Caroline> +                           struct dwp_file *dwp_file,
Caroline> +                           uint32_t unit_index,
Caroline> +                           const char *comp_dir,
Caroline> +                           ULONGEST signature, int is_debug_types)
Caroline> +{

...

Caroline> +  const struct dwp_hash_table *dwp_htab =
Caroline> +    is_debug_types ? dwp_file->tus : dwp_file->cus;

'=' on the wrong line.

Caroline> +  int i;

This should also be in the 'for' loop below.

Caroline> +  memset (&sections, 0, sizeof (sections));

Can this just be

    struct virtual_v2_or_v5_dwo_sections sections {};

?

gdb is moving more to a C++-y style where variables are declared near
their first use.

Caroline> +      dwo_file->dwo_name = obstack_strdup (&objfile->objfile_obstack,
Caroline> +                                           virtual_dwo_name);

Here we use objfile::intern now instead.  This matters because it puts
the string into the per-BFD object rather than the objfile obstack, so
that sharing works correctly.  I suspect this approach could cause
crashes in some multi-inferior scenarios.

Caroline> +  dwo_unit = OBSTACK_ZALLOC (&objfile->objfile_obstack, struct dwo_unit);

These were also moved to the per-BFD obstack during the big sharing
series.  The other site doing this now looks like:

  dwo_unit = OBSTACK_ZALLOC (&per_objfile->per_bfd->obstack, struct dwo_unit);

Caroline> +  dwo_unit->section =
Caroline> +    XOBNEW (&objfile->objfile_obstack, struct dwarf2_section_info);

Likewise.

Caroline> +	  else // version == 5

Comment style.

Tom