Re: [PATCH] Don't check PST is NULL in read_symtab

[Joel Brobecker <brobecker@adacore.com>]

[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]

> IMO, we don't need an assertion to check PST, because the function is
> used in this way,
> 
>   (*pst->read_symtab) (objfile, pst);

I am fine without the assertion as well. But if we followed your
argument, we would never need an assertion. For me, assertions
achieve two goals:
  1. Clearly document an expectation;
  2. Cause a semi-friendly abortion, rather than a mysterious behavior
     or crash.
As of today, the way this function is called indeed guarantees that
PST is never NULL. But someone adding a call at a later date might
introduce a bug and cause it to be called with PST == NULL...

> 2013-01-11  Yao Qi  <yao@codesourcery.com>
> 
> 	* dwarf2read.c (dwarf2_psymtab_to_symtab): Code indent.

Let's take an example to show you what I meant in my first suggestion.
Attached is a bogus change I made a function in dwarf2read. I added
an "else" around a large block of code. The first patch shows the
actual change, with code reindentation. That's the real patch which
would be checked in eventually - but it's barely readable. So,
to better show the real changes, I also attach the result of
"git diff/show -b", where whitespace-only changes are ignored.

-- 
Joel

[-- Attachment #2: dwarf2read-actual.diff --]
[-- Type: text/x-diff, Size: 2678 bytes --]

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index e2088f1..d590248 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1918,39 +1918,41 @@ dwarf2_read_section (struct objfile *objfile, struct dwarf2_section_info *info)
 
   if (dwarf2_section_empty_p (info))
     return;
+  else
+    {
+      abfd = sectp->owner;
 
-  abfd = sectp->owner;
+      /* If the section has relocations, we must read it ourselves.
+	 Otherwise we attach it to the BFD.  */
+      if ((sectp->flags & SEC_RELOC) == 0)
+	{
+	  const gdb_byte *bytes = gdb_bfd_map_section (sectp, &info->size);
 
-  /* If the section has relocations, we must read it ourselves.
-     Otherwise we attach it to the BFD.  */
-  if ((sectp->flags & SEC_RELOC) == 0)
-    {
-      const gdb_byte *bytes = gdb_bfd_map_section (sectp, &info->size);
+	  /* We have to cast away const here for historical reasons.
+	     Fixing dwarf2read to be const-correct would be quite nice.  */
+	  info->buffer = (gdb_byte *) bytes;
+	  return;
+	}
 
-      /* We have to cast away const here for historical reasons.
-	 Fixing dwarf2read to be const-correct would be quite nice.  */
-      info->buffer = (gdb_byte *) bytes;
-      return;
-    }
+      buf = obstack_alloc (&objfile->objfile_obstack, info->size);
+      info->buffer = buf;
 
-  buf = obstack_alloc (&objfile->objfile_obstack, info->size);
-  info->buffer = buf;
+      /* When debugging .o files, we may need to apply relocations; see
+	 http://sourceware.org/ml/gdb-patches/2002-04/msg00136.html .
+	 We never compress sections in .o files, so we only need to
+	 try this when the section is not compressed.  */
+      retbuf = symfile_relocate_debug_section (objfile, sectp, buf);
+      if (retbuf != NULL)
+	{
+	  info->buffer = retbuf;
+	  return;
+	}
 
-  /* When debugging .o files, we may need to apply relocations; see
-     http://sourceware.org/ml/gdb-patches/2002-04/msg00136.html .
-     We never compress sections in .o files, so we only need to
-     try this when the section is not compressed.  */
-  retbuf = symfile_relocate_debug_section (objfile, sectp, buf);
-  if (retbuf != NULL)
-    {
-      info->buffer = retbuf;
-      return;
+      if (bfd_seek (abfd, sectp->filepos, SEEK_SET) != 0
+	  || bfd_bread (buf, info->size, abfd) != info->size)
+	error (_("Dwarf Error: Can't read DWARF data from '%s'"),
+	       bfd_get_filename (abfd));
     }
-
-  if (bfd_seek (abfd, sectp->filepos, SEEK_SET) != 0
-      || bfd_bread (buf, info->size, abfd) != info->size)
-    error (_("Dwarf Error: Can't read DWARF data from '%s'"),
-	   bfd_get_filename (abfd));
 }
 
 /* A helper function that returns the size of a section in a safe way.

[-- Attachment #3: dwarf2read-spaces_ignored.diff --]
[-- Type: text/x-diff, Size: 733 bytes --]

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index e2088f1..d590248 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1918,7 +1918,8 @@ dwarf2_read_section (struct objfile *objfile, struct dwarf2_section_info *info)
 
   if (dwarf2_section_empty_p (info))
     return;
-
+  else
+    {
       abfd = sectp->owner;
 
       /* If the section has relocations, we must read it ourselves.
@@ -1951,6 +1952,7 @@ dwarf2_read_section (struct objfile *objfile, struct dwarf2_section_info *info)
 	  || bfd_bread (buf, info->size, abfd) != info->size)
 	error (_("Dwarf Error: Can't read DWARF data from '%s'"),
 	       bfd_get_filename (abfd));
+    }
 }
 
 /* A helper function that returns the size of a section in a safe way.