[PATCH 1/4] elfcompress: Pull set_section() into file scope

[PATCH 1/4] elfcompress: Pull set_section() into file scope

[tbaeder]

From: Timm Bäder <tbaeder@redhat.com>

Get rid of a nested function this way.

Signed-off-by: Timm Bäder <tbaeder@redhat.com>
---
 src/elfcompress.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/elfcompress.c b/src/elfcompress.c
index 1b5b1e36..65a922a7 100644
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -52,6 +52,8 @@ static const char *foutput = NULL;
 #define T_DECOMPRESS 1    /* none */
 #define T_COMPRESS_ZLIB 2 /* zlib */
 #define T_COMPRESS_GNU  3 /* zlib-gnu */
+#define WORD_BITS (8U * sizeof (unsigned int))
+
 static int type = T_UNSET;
 
 struct section_pattern
@@ -242,6 +244,12 @@ compress_section (Elf_Scn *scn, size_t orig_size, const char *name,
   return res;
 }
 
+static void
+set_section (unsigned int *sections, size_t ndx)
+{
+  sections[ndx / WORD_BITS] |= (1U << (ndx % WORD_BITS));
+}
+
 static int
 process_file (const char *fname)
 {
@@ -275,12 +283,6 @@ process_file (const char *fname)
   /* How many sections are we talking about?  */
   size_t shnum = 0;
 
-#define WORD_BITS (8U * sizeof (unsigned int))
-  void set_section (size_t ndx)
-  {
-    sections[ndx / WORD_BITS] |= (1U << (ndx % WORD_BITS));
-  }
-
   bool get_section (size_t ndx)
   {
     return (sections[ndx / WORD_BITS] & (1U << (ndx % WORD_BITS))) != 0;
@@ -498,7 +500,7 @@ process_file (const char *fname)
 	  else if (shdr->sh_type != SHT_NOBITS
 	      && (shdr->sh_flags & SHF_ALLOC) == 0)
 	    {
-	      set_section (ndx);
+	      set_section (sections, ndx);
 	      /* Check if we might want to change this section name.  */
 	      if (! adjust_names
 		  && ((type != T_COMPRESS_GNU
-- 
2.26.2

[PATCH 2/4] elfcompress: Pull get_section() into file scope

[tbaeder]

From: Timm Bäder <tbaeder@redhat.com>

Get rid of a nested function this way.

Signed-off-by: Timm Bäder <tbaeder@redhat.com>
---
 src/elfcompress.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/elfcompress.c b/src/elfcompress.c
index 65a922a7..2dc74a0c 100644
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -250,6 +250,12 @@ set_section (unsigned int *sections, size_t ndx)
   sections[ndx / WORD_BITS] |= (1U << (ndx % WORD_BITS));
 }
 
+static bool
+get_section (unsigned int *sections, size_t ndx)
+{
+  return (sections[ndx / WORD_BITS] & (1U << (ndx % WORD_BITS))) != 0;
+}
+
 static int
 process_file (const char *fname)
 {
@@ -283,11 +289,6 @@ process_file (const char *fname)
   /* How many sections are we talking about?  */
   size_t shnum = 0;
 
-  bool get_section (size_t ndx)
-  {
-    return (sections[ndx / WORD_BITS] & (1U << (ndx % WORD_BITS))) != 0;
-  }
-
   /* How many sections are we going to change?  */
   size_t get_sections (void)
   {
@@ -689,7 +690,7 @@ process_file (const char *fname)
       /* (de)compress if section matched.  */
       char *sname = NULL;
       char *newname = NULL;
-      if (get_section (ndx))
+      if (get_section (sections, ndx))
 	{
 	  GElf_Shdr shdr_mem;
 	  GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem);
-- 
2.26.2

[PATCH 3/4] elfcompress: Pull get_sections() into file scope

[tbaeder]

From: Timm Bäder <tbaeder@redhat.com>

Get rid of a nested function this way.

Signed-off-by: Timm Bäder <tbaeder@redhat.com>
---
 src/elfcompress.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/elfcompress.c b/src/elfcompress.c
index 2dc74a0c..65e28f0e 100644
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -256,6 +256,16 @@ get_section (unsigned int *sections, size_t ndx)
   return (sections[ndx / WORD_BITS] & (1U << (ndx % WORD_BITS))) != 0;
 }
 
+/* How many sections are we going to change?  */
+static size_t
+get_sections (unsigned int *sections, size_t shnum)
+{
+  size_t s = 0;
+  for (size_t i = 0; i < shnum / WORD_BITS + 1; i++)
+    s += __builtin_popcount (sections[i]);
+  return s;
+}
+
 static int
 process_file (const char *fname)
 {
@@ -289,15 +299,6 @@ process_file (const char *fname)
   /* How many sections are we talking about?  */
   size_t shnum = 0;
 
-  /* How many sections are we going to change?  */
-  size_t get_sections (void)
-  {
-    size_t s = 0;
-    for (size_t i = 0; i < shnum / WORD_BITS + 1; i++)
-      s += __builtin_popcount (sections[i]);
-    return s;
-  }
-
   int cleanup (int res)
   {
     elf_end (elf);
@@ -552,7 +553,7 @@ process_file (const char *fname)
 	  }
     }
 
-  if (foutput == NULL && get_sections () == 0)
+  if (foutput == NULL && get_sections (sections, shnum) == 0)
     {
       if (verbose > 0)
 	printf ("Nothing to do.\n");
-- 
2.26.2

[PATCH 4/4] elfcompress: Replace cleanup() with label

[tbaeder]

From: Timm Bäder <tbaeder@redhat.com>

Get rid of a nested function this way.

Signed-off-by: Timm Bäder <tbaeder@redhat.com>
---
 src/elfcompress.c | 218 +++++++++++++++++++++++-----------------------
 1 file changed, 109 insertions(+), 109 deletions(-)

diff --git a/src/elfcompress.c b/src/elfcompress.c
index 65e28f0e..ba08e73f 100644
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -298,47 +298,13 @@ process_file (const char *fname)
 
   /* How many sections are we talking about?  */
   size_t shnum = 0;
-
-  int cleanup (int res)
-  {
-    elf_end (elf);
-    close (fd);
-
-    elf_end (elfnew);
-    close (fdnew);
-
-    if (fnew != NULL)
-      {
-	unlink (fnew);
-	free (fnew);
-	fnew = NULL;
-      }
-
-    free (snamebuf);
-    if (names != NULL)
-      {
-	dwelf_strtab_free (names);
-	free (scnstrents);
-	free (symstrents);
-	free (namesbuf);
-	if (scnnames != NULL)
-	  {
-	    for (size_t n = 0; n < shnum; n++)
-	      free (scnnames[n]);
-	    free (scnnames);
-	  }
-      }
-
-    free (sections);
-
-    return res;
-  }
+  int res = 0;
 
   fd = open (fname, O_RDONLY);
   if (fd < 0)
     {
       error (0, errno, "Couldn't open %s\n", fname);
-      return cleanup (-1);
+      goto error;
     }
 
   elf = elf_begin (fd, ELF_C_READ, NULL);
@@ -346,7 +312,7 @@ process_file (const char *fname)
     {
       error (0, 0, "Couldn't open ELF file %s for reading: %s",
 	     fname, elf_errmsg (-1));
-      return cleanup (-1);
+      goto error;
     }
 
   /* We don't handle ar files (or anything else), we probably should.  */
@@ -357,21 +323,21 @@ process_file (const char *fname)
 	error (0, 0, "Cannot handle ar files: %s", fname);
       else
 	error (0, 0, "Unknown file type: %s", fname);
-      return cleanup (-1);
+      goto error;
     }
 
   struct stat st;
   if (fstat (fd, &st) != 0)
     {
       error (0, errno, "Couldn't fstat %s", fname);
-      return cleanup (-1);
+      goto error;
     }
 
   GElf_Ehdr ehdr;
   if (gelf_getehdr (elf, &ehdr) == NULL)
     {
       error (0, 0, "Couldn't get ehdr for %s: %s", fname, elf_errmsg (-1));
-      return cleanup (-1);
+      goto error;
     }
 
   /* Get the section header string table.  */
@@ -380,7 +346,7 @@ process_file (const char *fname)
     {
       error (0, 0, "Couldn't get section header string table index in %s: %s",
 	     fname, elf_errmsg (-1));
-      return cleanup (-1);
+      goto error;
     }
 
   /* How many sections are we talking about?  */
@@ -388,13 +354,13 @@ process_file (const char *fname)
     {
       error (0, 0, "Couldn't get number of sections in %s: %s",
 	     fname, elf_errmsg (1));
-      return cleanup (-1);
+      goto error;
     }
 
   if (shnum == 0)
     {
       error (0, 0, "ELF file %s has no sections", fname);
-      return cleanup (-1);
+      goto error;
     }
 
   sections = xcalloc (shnum / 8 + 1, sizeof (unsigned int));
@@ -403,7 +369,7 @@ process_file (const char *fname)
   if (elf_getphdrnum (elf, &phnum) != 0)
     {
       error (0, 0, "Couldn't get phdrnum: %s", elf_errmsg (-1));
-      return cleanup (-1);
+      goto error;
     }
 
   /* Whether we need to adjust any section names (going to/from GNU
@@ -460,7 +426,7 @@ process_file (const char *fname)
 	{
 	  error (0, 0, "Unexpected section number %zd, expected only %zd",
 		 ndx, shnum);
-	  cleanup (-1);
+	  goto error;
 	}
 
       GElf_Shdr shdr_mem;
@@ -468,14 +434,14 @@ process_file (const char *fname)
       if (shdr == NULL)
 	{
 	  error (0, 0, "Couldn't get shdr for section %zd", ndx);
-	  return cleanup (-1);
+	  goto error;
 	}
 
       const char *sname = elf_strptr (elf, shdrstrndx, shdr->sh_name);
       if (sname == NULL)
 	{
 	  error (0, 0, "Couldn't get name for section %zd", ndx);
-	  return cleanup (-1);
+	  goto error;
 	}
 
       if (section_name_matches (sname))
@@ -536,7 +502,7 @@ process_file (const char *fname)
 		{
 		  error (0, 0,
 			 "Multiple symbol tables (%zd, %zd) using the same string table unsupported", symtabndx, ndx);
-		  return cleanup (-1);
+		  goto error;
 		}
 	      symtabndx = ndx;
 	    }
@@ -558,7 +524,7 @@ process_file (const char *fname)
       if (verbose > 0)
 	printf ("Nothing to do.\n");
       fnew = NULL;
-      return cleanup (0);
+      goto cleanup;
     }
 
   if (adjust_names)
@@ -567,7 +533,7 @@ process_file (const char *fname)
       if (names == NULL)
 	{
 	  error (0, 0, "Not enough memory for new strtab");
-	  return cleanup (-1);
+	  goto error;
 	}
       scnstrents = xmalloc (shnum
 			    * sizeof (Dwelf_Strent *));
@@ -594,7 +560,7 @@ process_file (const char *fname)
       /* Since we didn't create it we don't want to try to unlink it.  */
       free (fnew);
       fnew = NULL;
-      return cleanup (-1);
+      goto error;
     }
 
   elfnew = elf_begin (fdnew, ELF_C_WRITE, NULL);
@@ -602,21 +568,21 @@ process_file (const char *fname)
     {
       error (0, 0, "Couldn't open new ELF %s for writing: %s",
 	     fnew, elf_errmsg (-1));
-      return cleanup (-1);
+      goto error;
     }
 
   /* Create the new ELF header and copy over all the data.  */
   if (gelf_newehdr (elfnew, gelf_getclass (elf)) == 0)
     {
       error (0, 0, "Couldn't create new ehdr: %s", elf_errmsg (-1));
-      return cleanup (-1);
+      goto error;
     }
 
   GElf_Ehdr newehdr;
   if (gelf_getehdr (elfnew, &newehdr) == NULL)
     {
       error (0, 0, "Couldn't get new ehdr: %s", elf_errmsg (-1));
-      return cleanup (-1);
+      goto error;
     }
 
   newehdr.e_ident[EI_DATA] = ehdr.e_ident[EI_DATA];
@@ -629,7 +595,7 @@ process_file (const char *fname)
   if (gelf_update_ehdr (elfnew, &newehdr) == 0)
     {
       error (0, 0, "Couldn't update ehdr: %s", elf_errmsg (-1));
-      return cleanup (-1);
+      goto error;
     }
 
   /* Copy over the phdrs as is.  */
@@ -638,7 +604,7 @@ process_file (const char *fname)
       if (gelf_newphdr (elfnew, phnum) == 0)
 	{
 	  error (0, 0, "Couldn't create phdrs: %s", elf_errmsg (-1));
-	  return cleanup (-1);
+	  goto error;
 	}
 
       for (size_t cnt = 0; cnt < phnum; ++cnt)
@@ -648,13 +614,13 @@ process_file (const char *fname)
 	  if (phdr == NULL)
 	    {
 	      error (0, 0, "Couldn't get phdr %zd: %s", cnt, elf_errmsg (-1));
-	      return cleanup (-1);
+	      goto error;
 	    }
 	  if (gelf_update_phdr (elfnew, cnt, phdr) == 0)
 	    {
 	      error (0, 0, "Couldn't create phdr %zd: %s", cnt,
 		     elf_errmsg (-1));
-	      return cleanup (-1);
+	      goto error;
 	    }
 	}
     }
@@ -698,7 +664,7 @@ process_file (const char *fname)
 	  if (shdr == NULL)
 	    {
 	      error (0, 0, "Couldn't get shdr for section %zd", ndx);
-	      return cleanup (-1);
+	      goto error;
 	    }
 
 	  uint64_t size = shdr->sh_size;
@@ -706,7 +672,7 @@ process_file (const char *fname)
 	  if (sname == NULL)
 	    {
 	      error (0, 0, "Couldn't get name for section %zd", ndx);
-	      return cleanup (-1);
+	      goto error;
 	    }
 
 	  /* strdup sname, the shdrstrndx section itself might be
@@ -728,7 +694,7 @@ process_file (const char *fname)
 		{
 		  if (compress_section (scn, size, sname, NULL, ndx,
 					false, false, verbose > 0) < 0)
-		    return cleanup (-1);
+		    goto error;
 		}
 	      else if (strncmp (sname, ".zdebug", strlen (".zdebug")) == 0)
 		{
@@ -737,7 +703,7 @@ process_file (const char *fname)
 		  newname = snamebuf;
 		  if (compress_section (scn, size, sname, newname, ndx,
 					true, false, verbose > 0) < 0)
-		    return cleanup (-1);
+		    goto error;
 		}
 	      else if (verbose > 0)
 		printf ("[%zd] %s already decompressed\n", ndx, sname);
@@ -752,7 +718,7 @@ process_file (const char *fname)
 			 Don't report even when verbose.  */
 		      if (compress_section (scn, size, sname, NULL, ndx,
 					    false, false, false) < 0)
-			return cleanup (-1);
+			goto error;
 		    }
 
 		  snamebuf[0] = '.';
@@ -779,13 +745,13 @@ process_file (const char *fname)
 		    }
 		  else
 		    {
-		      int res = compress_section (scn, size, sname, newname,
-						  ndx, true, true,
-						  verbose > 0);
-		      if (res < 0)
-			return cleanup (-1);
+		      int result = compress_section (scn, size, sname, newname,
+						     ndx, true, true,
+						     verbose > 0);
+		      if (result < 0)
+			goto error;
 
-		      if (res == 0)
+		      if (result == 0)
 			newname = NULL;
 		    }
 		}
@@ -809,7 +775,7 @@ process_file (const char *fname)
 			 Don't report even when verbose.  */
 		      if (compress_section (scn, size, sname, NULL, ndx,
 					    true, false, false) < 0)
-			return cleanup (-1);
+			goto error;
 
 		      snamebuf[0] = '.';
 		      strcpy (&snamebuf[1], &sname[2]);
@@ -837,7 +803,7 @@ process_file (const char *fname)
 		    }
 		  else if (compress_section (scn, size, sname, newname, ndx,
 					     false, true, verbose > 0) < 0)
-		    return cleanup (-1);
+		    goto error;
 		}
 	      else if (verbose > 0)
 		printf ("[%zd] %s already compressed\n", ndx, sname);
@@ -851,7 +817,7 @@ process_file (const char *fname)
       if (newscn == NULL)
 	{
 	  error (0, 0, "Couldn't create new section %zd", ndx);
-	  return cleanup (-1);
+	  goto error;
 	}
 
       GElf_Shdr shdr_mem;
@@ -859,13 +825,13 @@ process_file (const char *fname)
       if (shdr == NULL)
 	{
 	  error (0, 0, "Couldn't get shdr for section %zd", ndx);
-	  return cleanup (-1);
+	  goto error;
 	}
 
       if (gelf_update_shdr (newscn, shdr) == 0)
         {
 	  error (0, 0, "Couldn't update section header %zd", ndx);
-	  return cleanup (-1);
+	  goto error;
 	}
 
       /* Except for the section header string table all data can be
@@ -878,14 +844,14 @@ process_file (const char *fname)
 	  if (data == NULL)
 	    {
 	      error (0, 0, "Couldn't get data from section %zd", ndx);
-	      return cleanup (-1);
+	      goto error;
 	    }
 
 	  Elf_Data *newdata = elf_newdata (newscn);
 	  if (newdata == NULL)
 	    {
 	      error (0, 0, "Couldn't create new data for section %zd", ndx);
-	      return cleanup (-1);
+	      goto error;
 	    }
 
 	  *newdata = *data;
@@ -903,7 +869,7 @@ process_file (const char *fname)
 	      if (name == NULL)
 		{
 		  error (0, 0, "Couldn't get name for section [%zd]", ndx);
-		  return cleanup (-1);
+		  goto error;
 		}
 	    }
 
@@ -912,7 +878,7 @@ process_file (const char *fname)
 	  if ((scnstrents[ndx] = dwelf_strtab_add (names, name)) == NULL)
 	    {
 	      error (0, 0, "No memory to add section name string table");
-	      return cleanup (-1);
+	      goto error;
 	    }
 
 	  /* If the symtab shares strings then add those too.  */
@@ -929,7 +895,7 @@ process_file (const char *fname)
 		      /* Don't report the (internal) uncompression.  */
 		      if (compress_section (newscn, size, sname, NULL, ndx,
 					    false, false, false) < 0)
-			return cleanup (-1);
+			goto error;
 
 		      symtab_size = size;
 		      symtab_compressed = T_COMPRESS_ZLIB;
@@ -939,7 +905,7 @@ process_file (const char *fname)
 		      /* Don't report the (internal) uncompression.  */
 		      if (compress_section (newscn, size, sname, NULL, ndx,
 					    true, false, false) < 0)
-			return cleanup (-1);
+			goto error;
 
 		      symtab_size = size;
 		      symtab_compressed = T_COMPRESS_GNU;
@@ -951,7 +917,7 @@ process_file (const char *fname)
 		{
 		  error (0, 0, "Couldn't get symtab data for section [%zd] %s",
 			 ndx, name);
-		  return cleanup (-1);
+		  goto error;
 		}
 	      size_t elsize = gelf_fsize (elfnew, ELF_T_SYM, 1, EV_CURRENT);
 	      size_t syms = symd->d_size / elsize;
@@ -963,7 +929,7 @@ process_file (const char *fname)
 		  if (sym == NULL)
 		    {
 		      error (0, 0, "Couldn't get symbol %zd", i);
-		      return cleanup (-1);
+		      goto error;
 		    }
 		  if (sym->st_name != 0)
 		    {
@@ -975,13 +941,13 @@ process_file (const char *fname)
 		      if (symname == NULL)
 			{
 			  error (0, 0, "Couldn't get symbol %zd name", i);
-			  return cleanup (-1);
+			  goto error;
 			}
 		      symstrents[i] = dwelf_strtab_add (names, symname);
 		      if (symstrents[i] == NULL)
 			{
 			  error (0, 0, "No memory to add to symbol name");
-			  return cleanup (-1);
+			  goto error;
 			}
 		    }
 		}
@@ -1000,19 +966,19 @@ process_file (const char *fname)
 	{
 	  error (0, 0, "Couldn't get new section header string table [%zd]",
 		 shdrstrndx);
-	  return cleanup (-1);
+	  goto error;
 	}
 
       Elf_Data *data = elf_newdata (scn);
       if (data == NULL)
 	{
 	  error (0, 0, "Couldn't create new section header string table data");
-	  return cleanup (-1);
+	  goto error;
 	}
       if (dwelf_strtab_finalize (names, data) == NULL)
 	{
 	  error (0, 0, "Not enough memory to create string table");
-	  return cleanup (-1);
+	  goto error;
 	}
       namesbuf = data->d_buf;
 
@@ -1022,7 +988,7 @@ process_file (const char *fname)
 	{
 	  error (0, 0, "Couldn't get shdr for new section strings %zd",
 		 shdrstrndx);
-	  return cleanup (-1);
+	  goto error;
 	}
 
       /* Note that we also might have to compress and possibly set
@@ -1042,7 +1008,7 @@ process_file (const char *fname)
 	{
 	  error (0, 0, "Couldn't update new section strings [%zd]",
 		 shdrstrndx);
-	  return cleanup (-1);
+	  goto error;
 	}
 
       /* We might have to compress the data if the user asked us to,
@@ -1058,7 +1024,7 @@ process_file (const char *fname)
 	    {
 	      error (0, 0, "Couldn't get section header string table [%zd]",
 		     shdrstrndx);
-	      return cleanup (-1);
+	      goto error;
 	    }
 
 	  shdr = gelf_getshdr (oldscn, &shdr_mem);
@@ -1066,7 +1032,7 @@ process_file (const char *fname)
 	    {
 	      error (0, 0, "Couldn't get shdr for old section strings [%zd]",
 		     shdrstrndx);
-	      return cleanup (-1);
+	      goto error;
 	    }
 
 	  shstrtab_name = elf_strptr (elf, shdrstrndx, shdr->sh_name);
@@ -1074,7 +1040,7 @@ process_file (const char *fname)
 	    {
 	      error (0, 0, "Couldn't get name for old section strings [%zd]",
 		     shdrstrndx);
-	      return cleanup (-1);
+	      goto error;
 	    }
 
 	  shstrtab_size = shdr->sh_size;
@@ -1091,7 +1057,7 @@ process_file (const char *fname)
 				shstrtab_newname, shdrstrndx,
 				shstrtab_compressed == T_COMPRESS_GNU,
 				true, verbose > 0) < 0)
-	    return cleanup (-1);
+	    goto error;
 	}
     }
 
@@ -1100,7 +1066,7 @@ process_file (const char *fname)
   if (gelf_getehdr (elfnew, &newehdr) == NULL)
     {
       error (0, 0, "Couldn't re-get new ehdr: %s", elf_errmsg (-1));
-      return cleanup (-1);
+      goto error;
     }
 
   /* Set this after the sections have been created, otherwise section
@@ -1108,7 +1074,7 @@ process_file (const char *fname)
   if (setshdrstrndx (elfnew, &newehdr, shdrstrndx) != 0)
     {
       error (0, 0, "Couldn't set new shdrstrndx: %s", elf_errmsg (-1));
-      return cleanup (-1);
+      goto error;
     }
 
   /* Fixup pass.  Adjust string table references, symbol table and
@@ -1125,7 +1091,7 @@ process_file (const char *fname)
 	  if (shdr == NULL)
 	    {
 	      error (0, 0, "Couldn't get shdr for section %zd", ndx);
-	      return cleanup (-1);
+	      goto error;
 	    }
 
 	  /* Keep the offset of allocated sections so they are at the
@@ -1147,7 +1113,7 @@ process_file (const char *fname)
 	  if (gelf_update_shdr (scn, shdr) == 0)
 	    {
 	      error (0, 0, "Couldn't update section header %zd", ndx);
-	      return cleanup (-1);
+	      goto error;
 	    }
 
 	  if (adjust_names && ndx == symtabndx)
@@ -1160,7 +1126,7 @@ process_file (const char *fname)
 		{
 		  error (0, 0, "Couldn't get new symtab data section [%zd]",
 			 ndx);
-		  return cleanup (-1);
+		  goto error;
 		}
 	      size_t elsize = gelf_fsize (elfnew, ELF_T_SYM, 1, EV_CURRENT);
 	      size_t syms = symd->d_size / elsize;
@@ -1171,7 +1137,7 @@ process_file (const char *fname)
 		  if (sym == NULL)
 		    {
 		      error (0, 0, "2 Couldn't get symbol %zd", i);
-		      return cleanup (-1);
+		      goto error;
 		    }
 
 		  if (sym->st_name != 0)
@@ -1181,7 +1147,7 @@ process_file (const char *fname)
 		      if (gelf_update_sym (symd, i, sym) == 0)
 			{
 			  error (0, 0, "Couldn't update symbol %zd", i);
-			  return cleanup (-1);
+			  goto error;
 			}
 		    }
 		}
@@ -1199,7 +1165,7 @@ process_file (const char *fname)
 		    {
 		      error (0, 0, "Couldn't get symbol table [%zd]",
 			     symtabndx);
-		      return cleanup (-1);
+		      goto error;
 		    }
 
 		  shdr = gelf_getshdr (oldscn, &shdr_mem);
@@ -1207,7 +1173,7 @@ process_file (const char *fname)
 		    {
 		      error (0, 0, "Couldn't get old symbol table shdr [%zd]",
 			     symtabndx);
-		      return cleanup (-1);
+		      goto error;
 		    }
 
 		  symtab_name = elf_strptr (elf, shdrstrndx, shdr->sh_name);
@@ -1215,7 +1181,7 @@ process_file (const char *fname)
 		    {
 		      error (0, 0, "Couldn't get old symbol table name [%zd]",
 			     symtabndx);
-		      return cleanup (-1);
+		      goto error;
 		    }
 
 		  symtab_size = shdr->sh_size;
@@ -1233,7 +1199,7 @@ process_file (const char *fname)
 					symtab_newname, symtabndx,
 					symtab_compressed == T_COMPRESS_GNU,
 					true, verbose > 0) < 0)
-		    return cleanup (-1);
+		    goto error;
 		}
 	    }
 	}
@@ -1247,7 +1213,7 @@ process_file (const char *fname)
       if (gelf_getehdr (elfnew, &newehdr) == NULL)
 	{
 	  error (0, 0, "Couldn't get ehdr: %s", elf_errmsg (-1));
-	  return cleanup (-1);
+	  goto error;
 	}
 
       /* Position the shdrs after the last (unallocated) section.  */
@@ -1262,7 +1228,7 @@ process_file (const char *fname)
       if (gelf_update_ehdr (elfnew, &newehdr) == 0)
 	{
 	  error (0, 0, "Couldn't update ehdr: %s", elf_errmsg (-1));
-	  return cleanup (-1);
+	  goto error;
 	}
     }
 
@@ -1272,7 +1238,7 @@ process_file (const char *fname)
   if (elf_update (elfnew, ELF_C_WRITE) < 0)
     {
       error (0, 0, "Couldn't write %s: %s", fnew, elf_errmsg (-1));
-      return cleanup (-1);
+      goto error;
     }
 
   elf_end (elfnew);
@@ -1294,14 +1260,48 @@ process_file (const char *fname)
     if (rename (fnew, fname) != 0)
       {
 	error (0, errno, "Couldn't rename %s to %s", fnew, fname);
-	return cleanup (-1);
+	goto error;
       }
 
   /* We are finally done with the new file, don't unlink it now.  */
   free (fnew);
   fnew = NULL;
+  goto cleanup;
+
+error:
+  res = -1;
+
+cleanup:
+  elf_end (elf);
+  close (fd);
 
-  return cleanup (0);
+  elf_end (elfnew);
+  close (fdnew);
+
+  if (fnew != NULL)
+    {
+      unlink (fnew);
+      free (fnew);
+      fnew = NULL;
+    }
+
+  free (snamebuf);
+  if (names != NULL)
+    {
+      dwelf_strtab_free (names);
+      free (scnstrents);
+      free (symstrents);
+      free (namesbuf);
+      if (scnnames != NULL)
+	{
+	  for (size_t n = 0; n < shnum; n++)
+	    free (scnnames[n]);
+	  free (scnnames);
+	}
+    }
+
+  free (sections);
+  return res;
 }
 
 int
-- 
2.26.2

Re: [PATCH 1/4] elfcompress: Pull set_section() into file scope

[Mark Wielaard]

Hi Timm,

On Wed, 2021-02-17 at 09:45 +0100, Timm Bäder via Elfutils-devel wrote:
> Get rid of a nested function this way.

Added a ChangeLog entry and pushed.

Thanks,

Mark

Re: [PATCH 2/4] elfcompress: Pull get_section() into file scope

[Mark Wielaard]

Hi Timm,

On Wed, 2021-02-17 at 09:45 +0100, Timm Bäder via Elfutils-devel wrote:
> Get rid of a nested function this way.

Added a ChangeLog entry and pushed.

Thanks,

Mark

Re: [PATCH 3/4] elfcompress: Pull get_sections() into file scope

[Mark Wielaard]

Hi Timm,

On Wed, 2021-02-17 at 09:45 +0100, Timm Bäder via Elfutils-devel wrote:
> Get rid of a nested function this way.

Added a ChangeLog entry and pushed.

Thanks,

Mark

Re: [PATCH 4/4] elfcompress: Replace cleanup() with label

[Mark Wielaard]

Hi Timm,

On Wed, 2021-02-17 at 09:45 +0100, Timm Bäder via Elfutils-devel wrote:
> From: Timm Bäder <tbaeder@redhat.com>
> 
> Get rid of a nested function this way.

It would be nice to get a ChangeLog entry with this.

> diff --git a/src/elfcompress.c b/src/elfcompress.c
> index 65e28f0e..ba08e73f 100644
> --- a/src/elfcompress.c
> +++ b/src/elfcompress.c
> @@ -298,47 +298,13 @@ process_file (const char *fname)
>  
>    /* How many sections are we talking about?  */
>    size_t shnum = 0;
> -
> -  int cleanup (int res)
> -  {
> -    elf_end (elf);
> -    close (fd);
> -
> -    elf_end (elfnew);
> -    close (fdnew);
> -
> -    if (fnew != NULL)
> -      {
> -	unlink (fnew);
> -	free (fnew);
> -	fnew = NULL;
> -      }
> -
> -    free (snamebuf);
> -    if (names != NULL)
> -      {
> -	dwelf_strtab_free (names);
> -	free (scnstrents);
> -	free (symstrents);
> -	free (namesbuf);
> -	if (scnnames != NULL)
> -	  {
> -	    for (size_t n = 0; n < shnum; n++)
> -	      free (scnnames[n]);
> -	    free (scnnames);
> -	  }
> -      }
> -
> -    free (sections);
> -
> -    return res;
> -  }
> +  int res = 0;

If you are going to use a goto to the cleanup it makes sense to
initialize res to -1 here. Since till you get to the end, the result
will always be -1. And then you only need one (cleanup) label at the
end.

> @@ -1294,14 +1260,48 @@ process_file (const char *fname)
>      if (rename (fnew, fname) != 0)
>        {
>  	error (0, errno, "Couldn't rename %s to %s", fnew, fname);
> -	return cleanup (-1);
> +	goto error;
>        }
>  
>    /* We are finally done with the new file, don't unlink it now.  */
>    free (fnew);
>    fnew = NULL;
> +  goto cleanup;
> +
> +error:
> +  res = -1;

And then here instead of the goto cleanup and having an error label
simply set res = 0 and fall through to the cleanup label.

> +cleanup:
> +  elf_end (elf);
> +  close (fd);
>  
> -  return cleanup (0);
> +  elf_end (elfnew);
> +  close (fdnew);
> +
> +  if (fnew != NULL)
> +    {
> +      unlink (fnew);
> +      free (fnew);
> +      fnew = NULL;
> +    }
> +
> +  free (snamebuf);
> +  if (names != NULL)
> +    {
> +      dwelf_strtab_free (names);
> +      free (scnstrents);
> +      free (symstrents);
> +      free (namesbuf);
> +      if (scnnames != NULL)
> +	{
> +	  for (size_t n = 0; n < shnum; n++)
> +	    free (scnnames[n]);
> +	  free (scnnames);
> +	}
> +    }
> +
> +  free (sections);
> +  return res;
>  }

Cheers,

Mark