Re: [RFA take 6] Allow setting breakpoints on inline functions (PR 10738)

[Doug Evans <dje@google.com>]

On Wed, Mar 14, 2012 at 6:37 AM, Gary Benson <gbenson@redhat.com> wrote:
> Hi all,
>
> This patch makes GDB able to set breakpoints on inlined functions.
>
> This version of the patch has been updated to address Eli, Jan
> and Doug's comments on my previous version.  I have renamed the
> compatibility option from --allow-incomplete-gdb-indexes to
> --use-old-index-sections and documented the option in NEWS.  I
> also added a warning which will be printed the first time GDB
> finds an old .gdb_index section if --use-old-index-sections is
> not being used.  Note that I had to modify some tests to ignore
> these messages.  The documentation is essentially unchanged bar
> some specific changes Eli requested.
>
> How does this look?

Cool, thanks.

nit: ChangeLog description needs to be updated to use the renamed option.

+  if (use_old_index_sections)
+    {
+      if (version < 4)
+	return 0;
+    }
+  else
+    {
+      if (version < 6)
+	{
+	  static int warning_printed = 0;
+	  if (!warning_printed)
+	    {
+	      warning (_("Skipping old .gdb_index section in %s."),
+		       objfile->name);
+	      warning_printed = 1;
+	    }
+	  return 0;
+	}
+    }

If we're going to print warnings (and I think that's a great idea),
then we should do so consistently.
- for the "if (use_old_index_sections)" case, print warning "if (version < 4)"
- I think we should print different warnings for versions < 4, and versions 4,5.
  versions < 4: something like "Skipping obsolete .gdb-index section in %s" ?
  versions 4,5: something like "Skipping old .gdb_index section in %s,
pass --use-old-index-sections to use them anyway" ?
  [suitably formatted and worded]

   fputs_unfiltered (_("\
+  --use-old-index-sections\n\
+                     Do not reject old (broken) .gdb_index sections.\n\
+"), stream);

s/(broken)/(incomplete)/

Ok with me with those nits fixed.
[I didn't dig into the actual logic of the implementation, I'm
assuming that's already been vetted.]