[PATCH] [gdb] Refactor svr4_solib_ops::lm_addr_check

[Tom de Vries <tdevries@suse.de>]

I came across svr4_solib_ops::lm_addr_check and found it hard to follow.

In particular, it's indent-happy and minimizes code using a goto, resulting in
the following end-of-function:
...
            }
        }

    set_addr:
      li.l_addr = l_addr;
      li.l_addr_p = 1;
    }

  return li.l_addr;
}
...

I could just address the indentation, but I'd be adding a goto:
...
-      if (dynaddr + l_addr != l_dynaddr)
+      if (dynaddr + l_addr == l_dynaddr)
+        goto set_addr;
...

Instead, refactor the function into this form:
...
svr4_solib_ops::lm_addr_check (const solib &so, bfd *abfd) const
{
  auto &li = get_lm_info_svr4 (so);
  if (li.l_addr_p)
    return li.l_addr;

  auto set_addr = [&li] (CORE_ADDR addr)
  {
    li.l_addr = addr;
    li.l_addr_p = true;
    return addr;
  }

  if (...)
    return set_addr (l_addr);

  ...

  return set_addr (l_addr);
}
...
---
 gdb/solib-svr4.c | 160 +++++++++++++++++++++++------------------------
 1 file changed, 80 insertions(+), 80 deletions(-)

diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index c4af1b11a4d..40b0fc55113 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -225,106 +225,106 @@ CORE_ADDR
 svr4_solib_ops::lm_addr_check (const solib &so, bfd *abfd) const
 {
   auto &li = get_lm_info_svr4 (so);
+  if (li.l_addr_p)
+    return li.l_addr;
 
-  if (!li.l_addr_p)
-    {
-      struct bfd_section *dyninfo_sect;
-      CORE_ADDR l_addr, l_dynaddr, dynaddr;
+  auto set_addr = [&li] (CORE_ADDR addr)
+  {
+    li.l_addr = addr;
+    li.l_addr_p = true;
+    return addr;
+  };
 
-      l_addr = li.l_addr_inferior;
+  struct bfd_section *dyninfo_sect;
+  CORE_ADDR l_addr, l_dynaddr, dynaddr;
 
-      if (!abfd || !this->has_lm_dynamic_from_link_map ())
-	goto set_addr;
+  l_addr = li.l_addr_inferior;
+  if (!abfd || !this->has_lm_dynamic_from_link_map ())
+    return set_addr (l_addr);
 
-      l_dynaddr = li.l_ld;
+  l_dynaddr = li.l_ld;
 
-      dyninfo_sect = bfd_get_section_by_name (abfd, ".dynamic");
-      if (dyninfo_sect == NULL)
-	goto set_addr;
+  dyninfo_sect = bfd_get_section_by_name (abfd, ".dynamic");
+  if (dyninfo_sect == NULL)
+    return set_addr (l_addr);
 
-      dynaddr = bfd_section_vma (dyninfo_sect);
+  dynaddr = bfd_section_vma (dyninfo_sect);
+  if (dynaddr + l_addr == l_dynaddr)
+    return set_addr (l_addr);
 
-      if (dynaddr + l_addr != l_dynaddr)
-	{
-	  CORE_ADDR align = 0x1000;
-	  CORE_ADDR minpagesize = align;
+  CORE_ADDR align = 0x1000;
+  CORE_ADDR minpagesize = align;
 
-	  if (bfd_get_flavour (abfd) == bfd_target_elf_flavour)
-	    {
-	      Elf_Internal_Ehdr *ehdr = elf_tdata (abfd)->elf_header;
-	      Elf_Internal_Phdr *phdr = elf_tdata (abfd)->phdr;
-	      int i;
+  if (bfd_get_flavour (abfd) == bfd_target_elf_flavour)
+    {
+      Elf_Internal_Ehdr *ehdr = elf_tdata (abfd)->elf_header;
+      Elf_Internal_Phdr *phdr = elf_tdata (abfd)->phdr;
+      int i;
 
-	      align = 1;
+      align = 1;
 
-	      for (i = 0; i < ehdr->e_phnum; i++)
-		if (phdr[i].p_type == PT_LOAD && phdr[i].p_align > align)
-		  align = phdr[i].p_align;
+      for (i = 0; i < ehdr->e_phnum; i++)
+	if (phdr[i].p_type == PT_LOAD && phdr[i].p_align > align)
+	  align = phdr[i].p_align;
 
-	      minpagesize = get_elf_backend_data (abfd)->minpagesize;
-	    }
+      minpagesize = get_elf_backend_data (abfd)->minpagesize;
+    }
 
-	  /* Turn it into a mask.  */
-	  align--;
+  /* Turn it into a mask.  */
+  align--;
 
-	  /* If the changes match the alignment requirements, we
-	     assume we're using a core file that was generated by the
-	     same binary, just prelinked with a different base offset.
-	     If it doesn't match, we may have a different binary, the
-	     same binary with the dynamic table loaded at an unrelated
-	     location, or anything, really.  To avoid regressions,
-	     don't adjust the base offset in the latter case, although
-	     odds are that, if things really changed, debugging won't
-	     quite work.
+  /* If the changes match the alignment requirements, we
+     assume we're using a core file that was generated by the
+     same binary, just prelinked with a different base offset.
+     If it doesn't match, we may have a different binary, the
+     same binary with the dynamic table loaded at an unrelated
+     location, or anything, really.  To avoid regressions,
+     don't adjust the base offset in the latter case, although
+     odds are that, if things really changed, debugging won't
+     quite work.
 
-	     One could expect more the condition
-	       ((l_addr & align) == 0 && ((l_dynaddr - dynaddr) & align) == 0)
-	     but the one below is relaxed for PPC.  The PPC kernel supports
-	     either 4k or 64k page sizes.  To be prepared for 64k pages,
-	     PPC ELF files are built using an alignment requirement of 64k.
-	     However, when running on a kernel supporting 4k pages, the memory
-	     mapping of the library may not actually happen on a 64k boundary!
+     One could expect more the condition
+     ((l_addr & align) == 0 && ((l_dynaddr - dynaddr) & align) == 0)
+     but the one below is relaxed for PPC.  The PPC kernel supports
+     either 4k or 64k page sizes.  To be prepared for 64k pages,
+     PPC ELF files are built using an alignment requirement of 64k.
+     However, when running on a kernel supporting 4k pages, the memory
+     mapping of the library may not actually happen on a 64k boundary!
 
-	     (In the usual case where (l_addr & align) == 0, this check is
-	     equivalent to the possibly expected check above.)
+     (In the usual case where (l_addr & align) == 0, this check is
+     equivalent to the possibly expected check above.)
 
-	     Even on PPC it must be zero-aligned at least for MINPAGESIZE.  */
+     Even on PPC it must be zero-aligned at least for MINPAGESIZE.  */
 
-	  l_addr = l_dynaddr - dynaddr;
+  l_addr = l_dynaddr - dynaddr;
 
-	  if ((l_addr & (minpagesize - 1)) == 0
-	      && (l_addr & align) == ((l_dynaddr - dynaddr) & align))
-	    {
-	      if (info_verbose)
-		gdb_printf (_("Using PIC (Position Independent Code) "
-			      "prelink displacement %s for \"%s\".\n"),
-			    paddress (current_inferior ()->arch (), l_addr),
-			    so.name.c_str ());
-	    }
-	  else
-	    {
-	      /* There is no way to verify the library file matches.  prelink
-		 can during prelinking of an unprelinked file (or unprelinking
-		 of a prelinked file) shift the DYNAMIC segment by arbitrary
-		 offset without any page size alignment.  There is no way to
-		 find out the ELF header and/or Program Headers for a limited
-		 verification if it they match.  One could do a verification
-		 of the DYNAMIC segment.  Still the found address is the best
-		 one GDB could find.  */
-
-	      warning (_(".dynamic section for \"%s\" "
-			 "is not at the expected address "
-			 "(wrong library or version mismatch?)"),
-			 so.name.c_str ());
-	    }
-	}
-
-    set_addr:
-      li.l_addr = l_addr;
-      li.l_addr_p = 1;
+  if ((l_addr & (minpagesize - 1)) == 0
+      && (l_addr & align) == ((l_dynaddr - dynaddr) & align))
+    {
+      if (info_verbose)
+	gdb_printf (_("Using PIC (Position Independent Code) "
+		      "prelink displacement %s for \"%s\".\n"),
+		    paddress (current_inferior ()->arch (), l_addr),
+		    so.name.c_str ());
+    }
+  else
+    {
+      /* There is no way to verify the library file matches.  prelink
+	 can during prelinking of an unprelinked file (or unprelinking
+	 of a prelinked file) shift the DYNAMIC segment by arbitrary
+	 offset without any page size alignment.  There is no way to
+	 find out the ELF header and/or Program Headers for a limited
+	 verification if it they match.  One could do a verification
+	 of the DYNAMIC segment.  Still the found address is the best
+	 one GDB could find.  */
+
+      warning (_(".dynamic section for \"%s\" "
+		 "is not at the expected address "
+		 "(wrong library or version mismatch?)"),
+	       so.name.c_str ());
     }
 
-  return li.l_addr;
+  return set_addr (l_addr);
 }
 
 struct svr4_so

base-commit: 0855b93cfb911a4feab76a31bb1914982b3e6979
-- 
2.51.0