On Thu, Jun 18, 2020 at 1:27 PM Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Caroline" == Caroline Tice via Gdb-patches <
> gdb-patches@sourceware.org> writes:
>
> Thanks for the patch.
>
> Caroline> +          /* if (dwarf2_attr_no_follow (comp_unit_die,
> DW_AT_ranges)) */
>
> No need for commented-out code.
>
> Done.


> Caroline> +          if (comp_unit_die->attr (DW_AT_ranges) != nullptr)
> Caroline> +            {
> Caroline> +              cu->cu_ranges_from_skeleton = true;
> Caroline> +            }
>
> You can remove the braces here.  Normally in gdb there are only braces
> if there are multiple statements (or a comment).
>
> Done.


> Caroline> +  if (tag == DW_TAG_compile_unit &&
> Caroline> +      cu->cu_ranges_from_skeleton)
>
> The GNU / gdb style is to break before operators, not after (including
> assignment FWIW).  There are several occurrences of this, I didn't mark
> them all.
>
> Done.


> Caroline> +  if (section == nullptr)
> Caroline> +    error(_("Cannot find .debug_rnglists section [in module
> %s]"),
>
> Space before "(".
>
>
Done.

> Caroline> diff --git a/testsuite/gdb.dwarf2/dw5-rnglist-test.cc
> b/testsuite/gdb.dwarf2/dw5-rnglist-test.cc
> Caroline> new file mode 100644
> Caroline> index 0000000..4d650e1279
> Caroline> --- /dev/null
> Caroline> +++ b/testsuite/gdb.dwarf2/dw5-rnglist-test.cc
> Caroline> @@ -0,0 +1,71 @@
> Caroline> +#include <iostream>
>
> Tests also need the copyright header.
>
> Done.



> Caroline> diff --git a/testsuite/gdb.dwarf2/dw5-rnglist-test.exp
> b/testsuite/gdb.dwarf2/dw5-rnglist-test.exp
> Caroline> new file mode 100644
> Caroline> index 0000000..b5c6c3bfe3
> Caroline> --- /dev/null
> Caroline> +++ b/testsuite/gdb.dwarf2/dw5-rnglist-test.exp
> Caroline> @@ -0,0 +1,40 @@
> Caroline> +# Copyright 2011-2020 Free Software Foundation, Inc.
>
> This should probably just be 2020.
>
>
Done.

> Tom
>

I believe I have addressed all of your comments.  The updated patch is
attached. Please let me know if this is ok now.  Thanks!

-- Caroline Tice
cmtice@google.com