Misc single nested function removals

Misc single nested function removals

[tbaeder]

Hi,

here a few patches to remove single nested functions from

 - addr2line.c
 - tests/zstrptr.c
 - ar.c
 - arlib-argp.c

I think they are pretty straight-forward again.


- Timm

[PATCH 1/4] addr2line: Pull show_note() and show_int() in file scope

[tbaeder]

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

Get rid of the nested functions

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

diff --git a/src/addr2line.c b/src/addr2line.c
index ea01c1be..34945046 100644
--- a/src/addr2line.c
+++ b/src/addr2line.c
@@ -598,6 +598,26 @@ get_addr_width (Dwfl_Module *mod)
   return width;
 }
 
+static inline void
+show_note (int (*get) (Dwarf_Line *, bool *),
+	   Dwarf_Line *info,
+	   const char *note)
+{
+  bool flag;
+  if ((*get) (info, &flag) == 0 && flag)
+    fputs (note, stdout);
+}
+
+static inline void
+show_int (int (*get) (Dwarf_Line *, unsigned int *),
+	  Dwarf_Line *info,
+	  const char *name)
+{
+  unsigned int val;
+  if ((*get) (info, &val) == 0 && val != 0)
+    printf (" (%s %u)", name, val);
+}
+
 static int
 handle_address (const char *string, Dwfl *dwfl)
 {
@@ -692,27 +712,12 @@ handle_address (const char *string, Dwfl *dwfl)
 	  Dwarf_Line *info = dwfl_dwarf_line (line, &bias);
 	  assert (info != NULL);
 
-	  inline void show (int (*get) (Dwarf_Line *, bool *),
-			    const char *note)
-	  {
-	    bool flag;
-	    if ((*get) (info, &flag) == 0 && flag)
-	      fputs (note, stdout);
-	  }
-	  inline void show_int (int (*get) (Dwarf_Line *, unsigned int *),
-				const char *name)
-	  {
-	    unsigned int val;
-	    if ((*get) (info, &val) == 0 && val != 0)
-	      printf (" (%s %u)", name, val);
-	  }
-
-	  show (&dwarf_linebeginstatement, " (is_stmt)");
-	  show (&dwarf_lineblock, " (basic_block)");
-	  show (&dwarf_lineprologueend, " (prologue_end)");
-	  show (&dwarf_lineepiloguebegin, " (epilogue_begin)");
-	  show_int (&dwarf_lineisa, "isa");
-	  show_int (&dwarf_linediscriminator, "discriminator");
+	  show_note (&dwarf_linebeginstatement, info, " (is_stmt)");
+	  show_note (&dwarf_lineblock, info, " (basic_block)");
+	  show_note (&dwarf_lineprologueend, info, " (prologue_end)");
+	  show_note (&dwarf_lineepiloguebegin, info, " (epilogue_begin)");
+	  show_int (&dwarf_lineisa, info, "isa");
+	  show_int (&dwarf_linediscriminator, info, "discriminator");
 	}
       putchar ('\n');
     }
-- 
2.26.2

Re: [PATCH 1/4] addr2line: Pull show_note() and show_int() in file scope

[Mark Wielaard]

Hi Timm,

On Fri, 2021-01-08 at 09:13 +0100, Timm Bäder via Elfutils-devel wrote:
> Get rid of the nested functions

OK. These nested functions don't actually capture anything from their
outerscope. So they can be moved to file scope without requiring any
extra arguments. Added a ChangeLog entry and pushed.

Thanks,

Mark

[PATCH 2/4] zstrptr: Pull print_string() 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>
---
 tests/zstrptr.c | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/tests/zstrptr.c b/tests/zstrptr.c
index 6d8e19f7..9fb42e28 100644
--- a/tests/zstrptr.c
+++ b/tests/zstrptr.c
@@ -30,6 +30,26 @@
 #include ELFUTILS_HEADER(elf)
 #include <gelf.h>
 
+static void
+print_strings (Elf_Scn *scn, Elf *elf, size_t ndx)
+{
+  GElf_Shdr shdr_mem;
+  GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem);
+
+  printf ("Strings in section %zd (%s):\n", ndx,
+	  ((shdr->sh_flags & SHF_COMPRESSED) != 0
+	   ? "compressed" : "uncompressed"));
+
+  size_t off = 0;
+  const char *str = elf_strptr (elf, ndx, off);
+  while (str != NULL)
+    {
+      printf ("[%zx] '%s'\n", off, str);
+      off += strlen (str) + 1;
+      str = elf_strptr (elf, ndx, off);
+    }
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -79,38 +99,19 @@ main (int argc, char *argv[])
       exit (1);
     }
 
-  void print_strings (void)
-  {
-    GElf_Shdr shdr_mem;
-    GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem);
-
-    printf ("Strings in section %zd (%s):\n", ndx,
-	    ((shdr->sh_flags & SHF_COMPRESSED) != 0
-	     ? "compressed" : "uncompressed"));
-
-    size_t off = 0;
-    const char *str = elf_strptr (elf, ndx, off);
-    while (str != NULL)
-      {
-	printf ("[%zx] '%s'\n", off, str);
-	off += strlen (str) + 1;
-	str = elf_strptr (elf, ndx, off);
-      }
-  }
-
   if (elf_compress (scn, ELFCOMPRESS_ZLIB, 0) < 0)
     {
       printf ("Couldn't compress section %zd: %s\n", ndx, elf_errmsg (-1));
       exit (1);
     }
-  print_strings ();
+  print_strings (scn, elf, ndx);
 
   if (elf_compress (scn, 0, 0) < 0)
     {
       printf ("Couldn't decompress section %zd: %s\n", ndx, elf_errmsg (-1));
       exit (1);
     }
-  print_strings ();
+  print_strings (scn, elf, ndx);
 
   if (elf_end (elf) != 0)
     {
-- 
2.26.2

Re: [PATCH 2/4] zstrptr: Pull print_string() into file scope

[Mark Wielaard]

Hi Timm,

On Fri, 2021-01-08 at 09:13 +0100, Timm Bäder via Elfutils-devel wrote:
> Get rid of a nested function this way.

OK. This doesn't immediately make clear that the given scn and ndx
refer to the same section. But it does test the same thing (that given
section strings can be accessed through elf_strptr whether or not that
section is compressed.

Added a ChangeLog entry and pushed.

Thanks,

Mark

[PATCH 3/4] ar: Pull should_truncate_fname() 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/ar.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/ar.c b/src/ar.c
index 2a17d0d3..7d0298dd 100644
--- a/src/ar.c
+++ b/src/ar.c
@@ -436,6 +436,21 @@ copy_content (Elf *elf, int newfd, off_t off, size_t n)
   return write_retry (newfd, rawfile + off, n) != (ssize_t) n;
 }
 
+static inline bool
+should_truncate_fname (size_t name_max)
+{
+  if (errno == ENAMETOOLONG && allow_truncate_fname)
+    {
+      if (name_max == 0)
+	{
+	  long int len = pathconf (".", _PC_NAME_MAX);
+	  if (len > 0)
+	    name_max = len;
+	}
+      return name_max != 0;
+    }
+  return false;
+}
 
 static int
 do_oper_extract (int oper, const char *arfname, char **argv, int argc,
@@ -445,21 +460,6 @@ do_oper_extract (int oper, const char *arfname, char **argv, int argc,
   memset (found, '\0', sizeof (found));
 
   size_t name_max = 0;
-  inline bool should_truncate_fname (void)
-  {
-    if (errno == ENAMETOOLONG && allow_truncate_fname)
-      {
-	if (name_max == 0)
-	  {
-	    long int len = pathconf (".", _PC_NAME_MAX);
-	    if (len > 0)
-	      name_max = len;
-	  }
-	return name_max != 0;
-      }
-    return false;
-  }
-
   off_t index_off = -1;
   size_t index_size = 0;
   off_t cur_off = SARMAG;
@@ -615,7 +615,7 @@ do_oper_extract (int oper, const char *arfname, char **argv, int argc,
 		    {
 		      int printlen = INT_MAX;
 
-		      if (should_truncate_fname ())
+		      if (should_truncate_fname (name_max))
 			{
 			  /* Try to truncate the name.  First find out by how
 			     much.  */
@@ -704,7 +704,7 @@ do_oper_extract (int oper, const char *arfname, char **argv, int argc,
 		    {
 		      int printlen = INT_MAX;
 
-		      if (should_truncate_fname ())
+		      if (should_truncate_fname (name_max))
 			{
 			  /* Try to truncate the name.  First find out by how
 			     much.  */
-- 
2.26.2

Re: [PATCH 3/4] ar: Pull should_truncate_fname() into file scope

[Mark Wielaard]

Hi Timm,

On Fri, 2021-01-08 at 09:13 +0100, Timm Bäder via Elfutils-devel wrote:
> Get rid of a nested function this way.

Skipping this one for now since I don't believe I understand this code.
Could you explain what the code does what it does and why it works fine
when moved this way?

Thanks,

Mark

Re: [PATCH 3/4] ar: Pull should_truncate_fname() into file scope

[Timm Bäder]

On 29/01/2021 21:48, Mark Wielaard wrote:
> Hi Timm,
> 
> On Fri, 2021-01-08 at 09:13 +0100, Timm Bäder via Elfutils-devel wrote:
>> Get rid of a nested function this way.
> 
> Skipping this one for now since I don't believe I understand this code.
> Could you explain what the code does what it does and why it works fine
> when moved this way?

Right, I guess you mean because the old code used to assign a new value
to name_max, but the new code doesn't do that? I fixed that locally
now by taking a size_t* inout argument instead, so assigning to
name_max should have the desired effect again.


- Timm


-- 
Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Michael O'Neill, Tom Savage, Eric 
Shander

[PATCH 4/4] arlib-argp: Pull text_for_default() 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/arlib-argp.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/arlib-argp.c b/src/arlib-argp.c
index c07d9299..a3c12e4d 100644
--- a/src/arlib-argp.c
+++ b/src/arlib-argp.c
@@ -57,25 +57,26 @@ parse_opt (int key, char *arg __attribute__ ((unused)),
 }
 
 static char *
-help_filter (int key, const char *text, void *input __attribute__ ((unused)))
+text_for_default (const char *text)
 {
-  inline char *text_for_default (void)
-  {
-    char *new_text;
-    if (unlikely (asprintf (&new_text, _("%s (default)"), text) < 0))
-      return (char *) text;
-    return new_text;
-  }
+  char *new_text;
+  if (unlikely (asprintf (&new_text, _("%s (default)"), text) < 0))
+    return (char *) text;
+  return new_text;
+}
 
+static char *
+help_filter (int key, const char *text, void *input __attribute__ ((unused)))
+{
   switch (key)
     {
     case 'D':
       if (DEFAULT_AR_DETERMINISTIC)
-        return text_for_default ();
+        return text_for_default (text);
       break;
     case 'U':
       if (! DEFAULT_AR_DETERMINISTIC)
-        return text_for_default ();
+        return text_for_default (text);
       break;
     }
 
-- 
2.26.2

Re: [PATCH 4/4] arlib-argp: Pull text_for_default() into file scope

[Mark Wielaard]

Hi Timm,

On Fri, 2021-01-08 at 09:13 +0100, Timm Bäder via Elfutils-devel wrote:
> Get rid of a nested function this way.

Right, the only context captured is the text string pointer which is
now passed as argument. Added a Changelog entry and pushed.

Thanks,

Mark