diff mbox series

[v1,05/14] clean-up readelf: simplify and flatten body of process_attributes

Message ID 20250227104811.178094-6-matthieu.longo@arm.com
State New
Headers
Series Several clean-ups and refactorings before a long patch series on AArch64 build attributes |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 fail Patch failed to apply

Commit Message

Matthieu Longo Feb. 27, 2025, 10:47 a.m. UTC 
  - use find_section_by_type() instead of a for-loop.
- reindent the whole function accordingly.
- move declaration of variables nearer from their usage.
- prune else branch by using a goto in the error case.
diff mbox series

Patch

diff --git a/binutils/readelf.c b/binutils/readelf.c
index 6d3ec65a8a1..878012da8f0 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -19268,42 +19268,32 @@  process_attributes (Filedata * filedata,
                    unsigned char * (* display_pub_attribute) (unsigned char *, const unsigned char * const),
                    unsigned char * (* display_proc_gnu_attribute) (unsigned char *, unsigned int, const unsigned char * const))
 {
-  Elf_Internal_Shdr * sect;
-  unsigned i;
-  bool res = true;
-
   /* Find the section header so that we get the size.  */
-  for (i = 0, sect = filedata->section_headers;
-       i < filedata->file_header.e_shnum;
-       i++, sect++)
-    {
-      unsigned char * contents;
-      unsigned char * p;
+  Elf_Internal_Shdr * sect = find_section_by_type (filedata, proc_type);
+  if (sect == NULL)
+    sect = find_section_by_type (filedata, SHT_GNU_ATTRIBUTES);

-      if (sect->sh_type != proc_type && sect->sh_type != SHT_GNU_ATTRIBUTES)
-       continue;
+  if (sect == NULL)
+    /* No section, exit without error.  */
+    return true;

-      contents = (unsigned char *) get_data (NULL, filedata, sect->sh_offset, 1,
-                                             sect->sh_size, _("attributes"));
+  unsigned char * contents = (unsigned char *)
+    get_data (NULL, filedata, sect->sh_offset, 1, sect->sh_size, _("attributes"));
   if (contents == NULL)
-       {
-         res = false;
-         continue;
-       }
+    return false;

-      p = contents;
+  bool res = true;
+  unsigned char * p = contents;
   /* The first character is the version of the attributes.
      Currently only version 1, (aka 'A') is recognised here.  */
   if (*p != 'A')
     {
       printf (_("Unknown attributes version '%c'(%d) - expecting 'A'\n"), *p, *p);
       res = false;
+      goto free_data;
     }
-      else
-       {
-         uint64_t section_len;

-         section_len = sect->sh_size - 1;
+  uint64_t section_len = sect->sh_size - 1;
   p++;

   while (section_len > 0)
@@ -19456,10 +19446,9 @@  process_attributes (Filedata * filedata,
            attr_len = 0;
        }
     }
-       }

+free_data:
   free (contents);
-    }

   return res;
 }
---
 binutils/readelf.c | 317 ++++++++++++++++++++++-----------------------
 1 file changed, 153 insertions(+), 164 deletions(-)

diff --git a/binutils/readelf.c b/binutils/readelf.c
index 6d3ec65a8a1..878012da8f0 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -19268,199 +19268,188 @@  process_attributes (Filedata * filedata,
 		    unsigned char * (* display_pub_attribute) (unsigned char *, const unsigned char * const),
 		    unsigned char * (* display_proc_gnu_attribute) (unsigned char *, unsigned int, const unsigned char * const))
 {
-  Elf_Internal_Shdr * sect;
-  unsigned i;
-  bool res = true;
-
   /* Find the section header so that we get the size.  */
-  for (i = 0, sect = filedata->section_headers;
-       i < filedata->file_header.e_shnum;
-       i++, sect++)
+  Elf_Internal_Shdr * sect = find_section_by_type (filedata, proc_type);
+  if (sect == NULL)
+    sect = find_section_by_type (filedata, SHT_GNU_ATTRIBUTES);
+
+  if (sect == NULL)
+    /* No section, exit without error.  */
+    return true;
+
+  unsigned char * contents = (unsigned char *)
+    get_data (NULL, filedata, sect->sh_offset, 1, sect->sh_size, _("attributes"));
+  if (contents == NULL)
+    return false;
+
+  bool res = true;
+  unsigned char * p = contents;
+  /* The first character is the version of the attributes.
+     Currently only version 1, (aka 'A') is recognised here.  */
+  if (*p != 'A')
     {
-      unsigned char * contents;
-      unsigned char * p;
+      printf (_("Unknown attributes version '%c'(%d) - expecting 'A'\n"), *p, *p);
+      res = false;
+      goto free_data;
+    }
 
-      if (sect->sh_type != proc_type && sect->sh_type != SHT_GNU_ATTRIBUTES)
-	continue;
+  uint64_t section_len = sect->sh_size - 1;
+  p++;
+
+  while (section_len > 0)
+    {
+      uint64_t attr_len;
+      unsigned int namelen;
+      bool public_section;
+      bool gnu_section;
 
-      contents = (unsigned char *) get_data (NULL, filedata, sect->sh_offset, 1,
-                                             sect->sh_size, _("attributes"));
-      if (contents == NULL)
+      if (section_len <= 4)
 	{
+	  error (_("Tag section ends prematurely\n"));
 	  res = false;
-	  continue;
+	  break;
 	}
+      attr_len = byte_get (p, 4);
+      p += 4;
 
-      p = contents;
-      /* The first character is the version of the attributes.
-	 Currently only version 1, (aka 'A') is recognised here.  */
-      if (*p != 'A')
+      if (attr_len > section_len)
 	{
-	  printf (_("Unknown attributes version '%c'(%d) - expecting 'A'\n"), *p, *p);
+	  error (_("Bad attribute length (%u > %u)\n"),
+		  (unsigned) attr_len, (unsigned) section_len);
+	  attr_len = section_len;
 	  res = false;
 	}
-      else
+      /* PR 17531: file: 001-101425-0.004  */
+      else if (attr_len < 5)
 	{
-	  uint64_t section_len;
+	  error (_("Attribute length of %u is too small\n"), (unsigned) attr_len);
+	  res = false;
+	  break;
+	}
 
-	  section_len = sect->sh_size - 1;
-	  p++;
+      section_len -= attr_len;
+      attr_len -= 4;
 
-	  while (section_len > 0)
-	    {
-	      uint64_t attr_len;
-	      unsigned int namelen;
-	      bool public_section;
-	      bool gnu_section;
+      namelen = strnlen ((char *) p, attr_len) + 1;
+      if (namelen == 0 || namelen >= attr_len)
+	{
+	  error (_("Corrupt attribute section name\n"));
+	  res = false;
+	  break;
+	}
 
-	      if (section_len <= 4)
-		{
-		  error (_("Tag section ends prematurely\n"));
-		  res = false;
-		  break;
-		}
-	      attr_len = byte_get (p, 4);
-	      p += 4;
+      printf (_("Attribute Section: "));
+      print_symbol_name (INT_MAX, (const char *) p);
+      putchar ('\n');
 
-	      if (attr_len > section_len)
-		{
-		  error (_("Bad attribute length (%u > %u)\n"),
-			  (unsigned) attr_len, (unsigned) section_len);
-		  attr_len = section_len;
-		  res = false;
-		}
-	      /* PR 17531: file: 001-101425-0.004  */
-	      else if (attr_len < 5)
-		{
-		  error (_("Attribute length of %u is too small\n"), (unsigned) attr_len);
-		  res = false;
-		  break;
-		}
+      if (public_name && streq ((char *) p, public_name))
+	public_section = true;
+      else
+	public_section = false;
 
-	      section_len -= attr_len;
-	      attr_len -= 4;
+      if (streq ((char *) p, "gnu"))
+	gnu_section = true;
+      else
+	gnu_section = false;
 
-	      namelen = strnlen ((char *) p, attr_len) + 1;
-	      if (namelen == 0 || namelen >= attr_len)
-		{
-		  error (_("Corrupt attribute section name\n"));
-		  res = false;
-		  break;
-		}
+      p += namelen;
+      attr_len -= namelen;
 
-	      printf (_("Attribute Section: "));
-	      print_symbol_name (INT_MAX, (const char *) p);
-	      putchar ('\n');
+      while (attr_len > 0 && p < contents + sect->sh_size)
+	{
+	  int tag;
+	  unsigned int val;
+	  uint64_t size;
+	  unsigned char * end;
 
-	      if (public_name && streq ((char *) p, public_name))
-		public_section = true;
-	      else
-		public_section = false;
+	  /* PR binutils/17531: Safe handling of corrupt files.  */
+	  if (attr_len < 6)
+	    {
+	      error (_("Unused bytes at end of section\n"));
+	      res = false;
+	      section_len = 0;
+	      break;
+	    }
 
-	      if (streq ((char *) p, "gnu"))
-		gnu_section = true;
-	      else
-		gnu_section = false;
+	  tag = *(p++);
+	  size = byte_get (p, 4);
+	  if (size > attr_len)
+	    {
+	      error (_("Bad subsection length (%u > %u)\n"),
+		      (unsigned) size, (unsigned) attr_len);
+	      res = false;
+	      size = attr_len;
+	    }
+	  /* PR binutils/17531: Safe handling of corrupt files.  */
+	  if (size < 6)
+	    {
+	      error (_("Bad subsection length (%u < 6)\n"),
+		      (unsigned) size);
+	      res = false;
+	      section_len = 0;
+	      break;
+	    }
 
-	      p += namelen;
-	      attr_len -= namelen;
+	  attr_len -= size;
+	  end = p + size - 1;
+	  assert (end <= contents + sect->sh_size);
+	  p += 4;
 
-	      while (attr_len > 0 && p < contents + sect->sh_size)
+	  switch (tag)
+	    {
+	    case 1:
+	      printf (_("File Attributes\n"));
+	      break;
+	    case 2:
+	      printf (_("Section Attributes:"));
+	      goto do_numlist;
+	    case 3:
+	      printf (_("Symbol Attributes:"));
+	      /* Fall through.  */
+	    do_numlist:
+	      for (;;)
 		{
-		  int tag;
-		  unsigned int val;
-		  uint64_t size;
-		  unsigned char * end;
-
-		  /* PR binutils/17531: Safe handling of corrupt files.  */
-		  if (attr_len < 6)
-		    {
-		      error (_("Unused bytes at end of section\n"));
-		      res = false;
-		      section_len = 0;
-		      break;
-		    }
-
-		  tag = *(p++);
-		  size = byte_get (p, 4);
-		  if (size > attr_len)
-		    {
-		      error (_("Bad subsection length (%u > %u)\n"),
-			      (unsigned) size, (unsigned) attr_len);
-		      res = false;
-		      size = attr_len;
-		    }
-		  /* PR binutils/17531: Safe handling of corrupt files.  */
-		  if (size < 6)
-		    {
-		      error (_("Bad subsection length (%u < 6)\n"),
-			      (unsigned) size);
-		      res = false;
-		      section_len = 0;
-		      break;
-		    }
-
-		  attr_len -= size;
-		  end = p + size - 1;
-		  assert (end <= contents + sect->sh_size);
-		  p += 4;
-
-		  switch (tag)
-		    {
-		    case 1:
-		      printf (_("File Attributes\n"));
-		      break;
-		    case 2:
-		      printf (_("Section Attributes:"));
-		      goto do_numlist;
-		    case 3:
-		      printf (_("Symbol Attributes:"));
-		      /* Fall through.  */
-		    do_numlist:
-		      for (;;)
-			{
-			  READ_ULEB (val, p, end);
-			  if (val == 0)
-			    break;
-			  printf (" %d", val);
-			}
-		      printf ("\n");
-		      break;
-		    default:
-		      printf (_("Unknown tag: %d\n"), tag);
-		      public_section = false;
-		      break;
-		    }
-
-		  if (public_section && display_pub_attribute != NULL)
-		    {
-		      while (p < end)
-			p = display_pub_attribute (p, end);
-		      assert (p == end);
-		    }
-		  else if (gnu_section && display_proc_gnu_attribute != NULL)
-		    {
-		      while (p < end)
-			p = display_gnu_attribute (p,
-						   display_proc_gnu_attribute,
-						   end);
-		      assert (p == end);
-		    }
-		  else if (p < end)
-		    {
-		      printf (_("  Unknown attribute:\n"));
-		      display_raw_attribute (p, end);
-		      p = end;
-		    }
-		  else
-		    attr_len = 0;
+		  READ_ULEB (val, p, end);
+		  if (val == 0)
+		    break;
+		  printf (" %d", val);
 		}
+	      printf ("\n");
+	      break;
+	    default:
+	      printf (_("Unknown tag: %d\n"), tag);
+	      public_section = false;
+	      break;
 	    }
-	}
 
-      free (contents);
+	  if (public_section && display_pub_attribute != NULL)
+	    {
+	      while (p < end)
+		p = display_pub_attribute (p, end);
+	      assert (p == end);
+	    }
+	  else if (gnu_section && display_proc_gnu_attribute != NULL)
+	    {
+	      while (p < end)
+		p = display_gnu_attribute (p,
+					   display_proc_gnu_attribute,
+					   end);
+	      assert (p == end);
+	    }
+	  else if (p < end)
+	    {
+	      printf (_("  Unknown attribute:\n"));
+	      display_raw_attribute (p, end);
+	      p = end;
+	    }
+	  else
+	    attr_len = 0;
+	}
     }
 
+free_data:
+  free (contents);
+
   return res;
 }