public inbox for elfutils@sourceware.org
 help / color / mirror / Atom feed
* Re: [PATCH] Move nested functions in link_map.c to file scope.
@ 2016-01-03 21:25 Mark Wielaard
  0 siblings, 0 replies; 3+ messages in thread
From: Mark Wielaard @ 2016-01-03 21:25 UTC (permalink / raw)
  To: elfutils-devel

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

On Tue, 2015-11-17 at 14:45 -0800, Chih-Hung Hsieh wrote:
> * In libdwfl/link_map.c, nested functions check64, check32,

For check64 and check32 the define plus static function extra args call
trick seems a good fit. I did push this part of the patch.

>   release_buffer, read_addrs, and consider_phdr are moved
>   to file scope to compile with clang.

But for the rest I feel it makes the code unnecessary messy. So I didn't
apply those. See below for some suggestions how to maybe get something
that doesn't create new functions with lots and lots of arguments (which
I find doesn't improve readability).

> +static inline int
> +do_release_buffer (int result, Dwfl *dwfl,
> +                   void **pbuffer, size_t *pbuffer_available,
> +                   void *memory_callback_arg,
> +                   Dwfl_Memory_Callback *memory_callback)
> +{
> +  if (*pbuffer != NULL)
> +    (void) (*memory_callback) (dwfl, -1, pbuffer, pbuffer_available, 0, 0,
> +			       memory_callback_arg);
> +  return result;
> +}
> +
> +#define release_buffer(result) \
> +  do_release_buffer (result, dwfl, &buffer, &buffer_available, \
> +                     memory_callback_arg, memory_callback)

This is just 3 lines, and it is either used as "return release_buffer
(result)" to cleanup and return a result or as release_buffer (0) just
to cleanup the buffer (if not NULL). It seems better to make the define
just the function body. Assuming your compiler does know about statement
expressions https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html
Otherwise maybe just split it in two. Don't use an argument, just use it
as "cleanup_buffer()" plus a separate return line.

> +static inline bool
> +do_read_addrs (GElf_Addr vaddr, size_t n, GElf_Addr *addrs,
> +               uint_fast8_t elfdata, GElf_Addr *pread_vaddr,
> +               int elfclass, Dwfl *dwfl,
> +               void **pbuffer, size_t *pbuffer_available,
> +               void *memory_callback_arg,
> +               Dwfl_Memory_Callback *memory_callback)
> +{
> +  size_t nb = n * addrsize (elfclass); /* Address words -> bytes to read.  */
> +
> +  /* Read a new buffer if the old one doesn't cover these words.  */
> +  if (*pbuffer == NULL
> +      || vaddr < *pread_vaddr
> +      || vaddr - *pread_vaddr + nb > *pbuffer_available)
> +    {
> +      do_release_buffer (0, dwfl, pbuffer, pbuffer_available,
> +                         memory_callback_arg, memory_callback);
> +
> +      *pread_vaddr = vaddr;
> +      int segndx = INTUSE(dwfl_addrsegment) (dwfl, vaddr, NULL);
> +      if (unlikely (segndx < 0)
> +	  || unlikely (! (*memory_callback) (dwfl, segndx,
> +					     pbuffer, pbuffer_available,
> +					     vaddr, nb, memory_callback_arg)))
> +	return true;
> +    }
> +
> +  Elf32_Addr (*a32)[n] = vaddr - *pread_vaddr + *pbuffer;
> +  Elf64_Addr (*a64)[n] = (void *) a32;
> +
> +  if (elfclass == ELFCLASS32)
> +    {
> +      if (elfdata == ELFDATA2MSB)
> +	for (size_t i = 0; i < n; ++i)
> +	  addrs[i] = BE32 (read_4ubyte_unaligned_noncvt (&(*a32)[i]));
> +      else
> +	for (size_t i = 0; i < n; ++i)
> +	  addrs[i] = LE32 (read_4ubyte_unaligned_noncvt (&(*a32)[i]));
> +    }
> +  else
> +    {
> +      if (elfdata == ELFDATA2MSB)
> +	for (size_t i = 0; i < n; ++i)
> +	  addrs[i] = BE64 (read_8ubyte_unaligned_noncvt (&(*a64)[i]));
> +      else
> +	for (size_t i = 0; i < n; ++i)
> +	  addrs[i] = LE64 (read_8ubyte_unaligned_noncvt (&(*a64)[i]));
> +    }
> +
> +  return false;
> +}
> +
> +#define read_addrs(vaddr, n) \
> +  do_read_addrs (vaddr, n, addrs, elfdata, &read_vaddr, elfclass, dwfl, \
> +                 &buffer, &buffer_available, \
> +                 memory_callback_arg, memory_callback)

This very generic, but only used twice. Once to read 1 address and once
to read 4 addresses. Can't we make this a little simpler? Maybe just let
it handle reading one address. It might be a bit more hairy than needs
to because of the extra arguments needed to pass to the buffer cleanup.

> +static inline bool
> +do_consider_phdr (GElf_Word type, GElf_Addr vaddr, GElf_Xword filesz,
> +                  Dwfl *dwfl, GElf_Addr phdr, GElf_Addr *pdyn_bias,
> +                  GElf_Addr *pdyn_vaddr, GElf_Xword *pdyn_filesz)
> +{
> +  switch (type)
> +    {
> +    case PT_PHDR:
> +      if (*pdyn_bias == (GElf_Addr) - 1
> +	  /* Do a sanity check on the putative address.  */
> +	  && ((vaddr & (dwfl->segment_align - 1))
> +	      == (phdr & (dwfl->segment_align - 1))))
> +	{
> +	  *pdyn_bias = phdr - vaddr;
> +	  return *pdyn_vaddr != 0;
> +	}
> +      break;
> +
> +    case PT_DYNAMIC:
> +      *pdyn_vaddr = vaddr;
> +      *pdyn_filesz = filesz;
> +      return *pdyn_bias != (GElf_Addr) - 1;
> +    }
> +
> +  return false;
> +}
> +
> +#define consider_phdr(type, vaddr, filesz) \
> +  do_consider_phdr (type, vaddr, filesz, \
> +                    dwfl, phdr, &dyn_bias, &dyn_vaddr, &dyn_filesz)

This is just used twice in the function. Might be simpler to just inline
it twice and simplifying the switch by an if type == PT_... check
instead of inventing a new function with 8 arguments.

Thanks,

Mark

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Move nested functions in link_map.c to file scope.
@ 2016-01-06 19:35 Chih-Hung Hsieh
  0 siblings, 0 replies; 3+ messages in thread
From: Chih-Hung Hsieh @ 2016-01-06 19:35 UTC (permalink / raw)
  To: elfutils-devel

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

Mark,

Thanks for pushing several of my patches during the holidays.
I have merged all of them and latest elfutils changes into Android open
source project.
We are getting closer to compile all needed elfutils files with clang/llvm
for Android.

I will continue to fix the remain elfutils issues after fixing a few other
urgent unrelated Android problems. Thanks.


On Sun, Jan 3, 2016 at 1:25 PM, Mark Wielaard <mjw@redhat.com> wrote:

> On Tue, 2015-11-17 at 14:45 -0800, Chih-Hung Hsieh wrote:
> > * In libdwfl/link_map.c, nested functions check64, check32,
>
> For check64 and check32 the define plus static function extra args call
> trick seems a good fit. I did push this part of the patch.
>
> >   release_buffer, read_addrs, and consider_phdr are moved
> >   to file scope to compile with clang.
>
> But for the rest I feel it makes the code unnecessary messy. So I didn't
> apply those. See below for some suggestions how to maybe get something
> that doesn't create new functions with lots and lots of arguments (which
> I find doesn't improve readability).
>
> > +static inline int
> > +do_release_buffer (int result, Dwfl *dwfl,
> > +                   void **pbuffer, size_t *pbuffer_available,
> > +                   void *memory_callback_arg,
> > +                   Dwfl_Memory_Callback *memory_callback)
> > +{
> > +  if (*pbuffer != NULL)
> > +    (void) (*memory_callback) (dwfl, -1, pbuffer, pbuffer_available, 0,
> 0,
> > +                            memory_callback_arg);
> > +  return result;
> > +}
> > +
> > +#define release_buffer(result) \
> > +  do_release_buffer (result, dwfl, &buffer, &buffer_available, \
> > +                     memory_callback_arg, memory_callback)
>
> This is just 3 lines, and it is either used as "return release_buffer
> (result)" to cleanup and return a result or as release_buffer (0) just
> to cleanup the buffer (if not NULL). It seems better to make the define
> just the function body. Assuming your compiler does know about statement
> expressions https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html
> Otherwise maybe just split it in two. Don't use an argument, just use it
> as "cleanup_buffer()" plus a separate return line.
>
> > +static inline bool
> > +do_read_addrs (GElf_Addr vaddr, size_t n, GElf_Addr *addrs,
> > +               uint_fast8_t elfdata, GElf_Addr *pread_vaddr,
> > +               int elfclass, Dwfl *dwfl,
> > +               void **pbuffer, size_t *pbuffer_available,
> > +               void *memory_callback_arg,
> > +               Dwfl_Memory_Callback *memory_callback)
> > +{
> > +  size_t nb = n * addrsize (elfclass); /* Address words -> bytes to
> read.  */
> > +
> > +  /* Read a new buffer if the old one doesn't cover these words.  */
> > +  if (*pbuffer == NULL
> > +      || vaddr < *pread_vaddr
> > +      || vaddr - *pread_vaddr + nb > *pbuffer_available)
> > +    {
> > +      do_release_buffer (0, dwfl, pbuffer, pbuffer_available,
> > +                         memory_callback_arg, memory_callback);
> > +
> > +      *pread_vaddr = vaddr;
> > +      int segndx = INTUSE(dwfl_addrsegment) (dwfl, vaddr, NULL);
> > +      if (unlikely (segndx < 0)
> > +       || unlikely (! (*memory_callback) (dwfl, segndx,
> > +                                          pbuffer, pbuffer_available,
> > +                                          vaddr, nb,
> memory_callback_arg)))
> > +     return true;
> > +    }
> > +
> > +  Elf32_Addr (*a32)[n] = vaddr - *pread_vaddr + *pbuffer;
> > +  Elf64_Addr (*a64)[n] = (void *) a32;
> > +
> > +  if (elfclass == ELFCLASS32)
> > +    {
> > +      if (elfdata == ELFDATA2MSB)
> > +     for (size_t i = 0; i < n; ++i)
> > +       addrs[i] = BE32 (read_4ubyte_unaligned_noncvt (&(*a32)[i]));
> > +      else
> > +     for (size_t i = 0; i < n; ++i)
> > +       addrs[i] = LE32 (read_4ubyte_unaligned_noncvt (&(*a32)[i]));
> > +    }
> > +  else
> > +    {
> > +      if (elfdata == ELFDATA2MSB)
> > +     for (size_t i = 0; i < n; ++i)
> > +       addrs[i] = BE64 (read_8ubyte_unaligned_noncvt (&(*a64)[i]));
> > +      else
> > +     for (size_t i = 0; i < n; ++i)
> > +       addrs[i] = LE64 (read_8ubyte_unaligned_noncvt (&(*a64)[i]));
> > +    }
> > +
> > +  return false;
> > +}
> > +
> > +#define read_addrs(vaddr, n) \
> > +  do_read_addrs (vaddr, n, addrs, elfdata, &read_vaddr, elfclass, dwfl,
> \
> > +                 &buffer, &buffer_available, \
> > +                 memory_callback_arg, memory_callback)
>
> This very generic, but only used twice. Once to read 1 address and once
> to read 4 addresses. Can't we make this a little simpler? Maybe just let
> it handle reading one address. It might be a bit more hairy than needs
> to because of the extra arguments needed to pass to the buffer cleanup.
>
> > +static inline bool
> > +do_consider_phdr (GElf_Word type, GElf_Addr vaddr, GElf_Xword filesz,
> > +                  Dwfl *dwfl, GElf_Addr phdr, GElf_Addr *pdyn_bias,
> > +                  GElf_Addr *pdyn_vaddr, GElf_Xword *pdyn_filesz)
> > +{
> > +  switch (type)
> > +    {
> > +    case PT_PHDR:
> > +      if (*pdyn_bias == (GElf_Addr) - 1
> > +       /* Do a sanity check on the putative address.  */
> > +       && ((vaddr & (dwfl->segment_align - 1))
> > +           == (phdr & (dwfl->segment_align - 1))))
> > +     {
> > +       *pdyn_bias = phdr - vaddr;
> > +       return *pdyn_vaddr != 0;
> > +     }
> > +      break;
> > +
> > +    case PT_DYNAMIC:
> > +      *pdyn_vaddr = vaddr;
> > +      *pdyn_filesz = filesz;
> > +      return *pdyn_bias != (GElf_Addr) - 1;
> > +    }
> > +
> > +  return false;
> > +}
> > +
> > +#define consider_phdr(type, vaddr, filesz) \
> > +  do_consider_phdr (type, vaddr, filesz, \
> > +                    dwfl, phdr, &dyn_bias, &dyn_vaddr, &dyn_filesz)
>
> This is just used twice in the function. Might be simpler to just inline
> it twice and simplifying the switch by an if type == PT_... check
> instead of inventing a new function with 8 arguments.
>
> Thanks,
>
> Mark
>
>

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 7673 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] Move nested functions in link_map.c to file scope.
@ 2015-11-17 22:45 Chih-Hung Hsieh
  0 siblings, 0 replies; 3+ messages in thread
From: Chih-Hung Hsieh @ 2015-11-17 22:45 UTC (permalink / raw)
  To: elfutils-devel

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

* In libdwfl/link_map.c, nested functions check64, check32,
  release_buffer, read_addrs, and consider_phdr are moved
  to file scope to compile with clang.

Signed-off-by: Chih-Hung Hsieh <chh@google.com>
---
 libdwfl/ChangeLog  |   9 ++
 libdwfl/link_map.c | 282 ++++++++++++++++++++++++++++++-----------------------
 2 files changed, 167 insertions(+), 124 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 163a6f1..c103bc9 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,12 @@
+2015-11-17  Chih-Hung Hsieh <chh@google.com>
+
+	* link_map.c (auxv_format_probe): Move nested functions
+	check64 and check32 to file scope.
+	* link_map.c (report_r_debug): Move nested functions
+	release_buffer and read_addrs to file scope.
+	* link_map.c (dwfl_lnk_map_report): Move nested function
+	consider_phdr to file scope.
+
 2015-11-13  Chih-Hung Hsieh <chh@google.com>
 
 	* gzip.c (unzip): Move nested functions to file scope.
diff --git a/libdwfl/link_map.c b/libdwfl/link_map.c
index 13cac52..46d677a 100644
--- a/libdwfl/link_map.c
+++ b/libdwfl/link_map.c
@@ -42,64 +42,70 @@
 #define PROBE_VAL64	sizeof (Elf64_Phdr)
 
 
-/* Examine an auxv data block and determine its format.
-   Return true iff we figured it out.  */
-static bool
-auxv_format_probe (const void *auxv, size_t size,
-		   uint_fast8_t *elfclass, uint_fast8_t *elfdata)
+static inline bool
+do_check64 (size_t i, const Elf64_auxv_t (*a64)[], uint_fast8_t *elfdata)
 {
-  const Elf32_auxv_t (*a32)[size / sizeof (Elf32_auxv_t)] = (void *) auxv;
-  const Elf64_auxv_t (*a64)[size / sizeof (Elf64_auxv_t)] = (void *) auxv;
+  /* The AUXV pointer might not even be naturally aligned for 64-bit
+     data, because note payloads in a core file are not aligned.  */
 
-  inline bool check64 (size_t i)
-  {
-    /* The AUXV pointer might not even be naturally aligned for 64-bit
-       data, because note payloads in a core file are not aligned.  */
+  uint64_t type = read_8ubyte_unaligned_noncvt (&(*a64)[i].a_type);
+  uint64_t val = read_8ubyte_unaligned_noncvt (&(*a64)[i].a_un.a_val);
 
-    uint64_t type = read_8ubyte_unaligned_noncvt (&(*a64)[i].a_type);
-    uint64_t val = read_8ubyte_unaligned_noncvt (&(*a64)[i].a_un.a_val);
+  if (type == BE64 (PROBE_TYPE)
+      && val == BE64 (PROBE_VAL64))
+    {
+      *elfdata = ELFDATA2MSB;
+      return true;
+    }
 
-    if (type == BE64 (PROBE_TYPE)
-	&& val == BE64 (PROBE_VAL64))
-      {
-	*elfdata = ELFDATA2MSB;
-	return true;
-      }
+  if (type == LE64 (PROBE_TYPE)
+      && val == LE64 (PROBE_VAL64))
+    {
+      *elfdata = ELFDATA2LSB;
+      return true;
+    }
 
-    if (type == LE64 (PROBE_TYPE)
-	&& val == LE64 (PROBE_VAL64))
-      {
-	*elfdata = ELFDATA2LSB;
-	return true;
-      }
+  return false;
+}
 
-    return false;
-  }
+#define check64(n) do_check64 (n, a64, elfdata)
 
-  inline bool check32 (size_t i)
-  {
-    /* The AUXV pointer might not even be naturally aligned for 32-bit
-       data, because note payloads in a core file are not aligned.  */
+static inline bool
+do_check32 (size_t i, const Elf32_auxv_t (*a32)[], uint_fast8_t *elfdata)
+{
+  /* The AUXV pointer might not even be naturally aligned for 32-bit
+     data, because note payloads in a core file are not aligned.  */
 
-    uint32_t type = read_4ubyte_unaligned_noncvt (&(*a32)[i].a_type);
-    uint32_t val = read_4ubyte_unaligned_noncvt (&(*a32)[i].a_un.a_val);
+  uint32_t type = read_4ubyte_unaligned_noncvt (&(*a32)[i].a_type);
+  uint32_t val = read_4ubyte_unaligned_noncvt (&(*a32)[i].a_un.a_val);
 
-    if (type == BE32 (PROBE_TYPE)
-	&& val == BE32 (PROBE_VAL32))
-      {
-	*elfdata = ELFDATA2MSB;
-	return true;
-      }
+  if (type == BE32 (PROBE_TYPE)
+      && val == BE32 (PROBE_VAL32))
+    {
+      *elfdata = ELFDATA2MSB;
+      return true;
+    }
 
-    if (type == LE32 (PROBE_TYPE)
-	&& val == LE32 (PROBE_VAL32))
-      {
-	*elfdata = ELFDATA2LSB;
-	return true;
-      }
+  if (type == LE32 (PROBE_TYPE)
+      && val == LE32 (PROBE_VAL32))
+    {
+      *elfdata = ELFDATA2LSB;
+      return true;
+    }
 
-    return false;
-  }
+  return false;
+}
+
+#define check32(n) do_check32 (n, a32, elfdata)
+
+/* Examine an auxv data block and determine its format.
+   Return true iff we figured it out.  */
+static bool
+auxv_format_probe (const void *auxv, size_t size,
+		   uint_fast8_t *elfclass, uint_fast8_t *elfdata)
+{
+  const Elf32_auxv_t (*a32)[size / sizeof (Elf32_auxv_t)] = (void *) auxv;
+  const Elf64_auxv_t (*a64)[size / sizeof (Elf64_auxv_t)] = (void *) auxv;
 
   for (size_t i = 0; i < size / sizeof (Elf64_auxv_t); ++i)
     {
@@ -223,6 +229,80 @@ addrsize (uint_fast8_t elfclass)
   return elfclass * 4;
 }
 
+static inline int
+do_release_buffer (int result, Dwfl *dwfl,
+                   void **pbuffer, size_t *pbuffer_available,
+                   void *memory_callback_arg,
+                   Dwfl_Memory_Callback *memory_callback)
+{
+  if (*pbuffer != NULL)
+    (void) (*memory_callback) (dwfl, -1, pbuffer, pbuffer_available, 0, 0,
+			       memory_callback_arg);
+  return result;
+}
+
+#define release_buffer(result) \
+  do_release_buffer (result, dwfl, &buffer, &buffer_available, \
+                     memory_callback_arg, memory_callback)
+
+static inline bool
+do_read_addrs (GElf_Addr vaddr, size_t n, GElf_Addr *addrs,
+               uint_fast8_t elfdata, GElf_Addr *pread_vaddr,
+               int elfclass, Dwfl *dwfl,
+               void **pbuffer, size_t *pbuffer_available,
+               void *memory_callback_arg,
+               Dwfl_Memory_Callback *memory_callback)
+{
+  size_t nb = n * addrsize (elfclass); /* Address words -> bytes to read.  */
+
+  /* Read a new buffer if the old one doesn't cover these words.  */
+  if (*pbuffer == NULL
+      || vaddr < *pread_vaddr
+      || vaddr - *pread_vaddr + nb > *pbuffer_available)
+    {
+      do_release_buffer (0, dwfl, pbuffer, pbuffer_available,
+                         memory_callback_arg, memory_callback);
+
+      *pread_vaddr = vaddr;
+      int segndx = INTUSE(dwfl_addrsegment) (dwfl, vaddr, NULL);
+      if (unlikely (segndx < 0)
+	  || unlikely (! (*memory_callback) (dwfl, segndx,
+					     pbuffer, pbuffer_available,
+					     vaddr, nb, memory_callback_arg)))
+	return true;
+    }
+
+  Elf32_Addr (*a32)[n] = vaddr - *pread_vaddr + *pbuffer;
+  Elf64_Addr (*a64)[n] = (void *) a32;
+
+  if (elfclass == ELFCLASS32)
+    {
+      if (elfdata == ELFDATA2MSB)
+	for (size_t i = 0; i < n; ++i)
+	  addrs[i] = BE32 (read_4ubyte_unaligned_noncvt (&(*a32)[i]));
+      else
+	for (size_t i = 0; i < n; ++i)
+	  addrs[i] = LE32 (read_4ubyte_unaligned_noncvt (&(*a32)[i]));
+    }
+  else
+    {
+      if (elfdata == ELFDATA2MSB)
+	for (size_t i = 0; i < n; ++i)
+	  addrs[i] = BE64 (read_8ubyte_unaligned_noncvt (&(*a64)[i]));
+      else
+	for (size_t i = 0; i < n; ++i)
+	  addrs[i] = LE64 (read_8ubyte_unaligned_noncvt (&(*a64)[i]));
+    }
+
+  return false;
+}
+
+#define read_addrs(vaddr, n) \
+  do_read_addrs (vaddr, n, addrs, elfdata, &read_vaddr, elfclass, dwfl, \
+                 &buffer, &buffer_available, \
+                 memory_callback_arg, memory_callback)
+
+
 /* Report a module for each struct link_map in the linked list at r_map
    in the struct r_debug at R_DEBUG_VADDR.  For r_debug_info description
    see dwfl_link_map_report in libdwflP.h.  If R_DEBUG_INFO is not NULL then no
@@ -247,59 +327,7 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
 
   void *buffer = NULL;
   size_t buffer_available = 0;
-  inline int release_buffer (int result)
-  {
-    if (buffer != NULL)
-      (void) (*memory_callback) (dwfl, -1, &buffer, &buffer_available, 0, 0,
-				 memory_callback_arg);
-    return result;
-  }
-
   GElf_Addr addrs[4];
-  inline bool read_addrs (GElf_Addr vaddr, size_t n)
-  {
-    size_t nb = n * addrsize (elfclass); /* Address words -> bytes to read.  */
-
-    /* Read a new buffer if the old one doesn't cover these words.  */
-    if (buffer == NULL
-	|| vaddr < read_vaddr
-	|| vaddr - read_vaddr + nb > buffer_available)
-      {
-	release_buffer (0);
-
-	read_vaddr = vaddr;
-	int segndx = INTUSE(dwfl_addrsegment) (dwfl, vaddr, NULL);
-	if (unlikely (segndx < 0)
-	    || unlikely (! (*memory_callback) (dwfl, segndx,
-					       &buffer, &buffer_available,
-					       vaddr, nb, memory_callback_arg)))
-	  return true;
-      }
-
-    Elf32_Addr (*a32)[n] = vaddr - read_vaddr + buffer;
-    Elf64_Addr (*a64)[n] = (void *) a32;
-
-    if (elfclass == ELFCLASS32)
-      {
-	if (elfdata == ELFDATA2MSB)
-	  for (size_t i = 0; i < n; ++i)
-	    addrs[i] = BE32 (read_4ubyte_unaligned_noncvt (&(*a32)[i]));
-	else
-	  for (size_t i = 0; i < n; ++i)
-	    addrs[i] = LE32 (read_4ubyte_unaligned_noncvt (&(*a32)[i]));
-      }
-    else
-      {
-	if (elfdata == ELFDATA2MSB)
-	  for (size_t i = 0; i < n; ++i)
-	    addrs[i] = BE64 (read_8ubyte_unaligned_noncvt (&(*a64)[i]));
-	else
-	  for (size_t i = 0; i < n; ++i)
-	    addrs[i] = LE64 (read_8ubyte_unaligned_noncvt (&(*a64)[i]));
-      }
-
-    return false;
-  }
 
   if (unlikely (read_addrs (read_vaddr, 1)))
     return release_buffer (-1);
@@ -684,6 +712,37 @@ find_executable (Dwfl *dwfl, GElf_Addr at_phdr, GElf_Addr at_entry,
 }
 \f

 
+static inline bool
+do_consider_phdr (GElf_Word type, GElf_Addr vaddr, GElf_Xword filesz,
+                  Dwfl *dwfl, GElf_Addr phdr, GElf_Addr *pdyn_bias,
+                  GElf_Addr *pdyn_vaddr, GElf_Xword *pdyn_filesz)
+{
+  switch (type)
+    {
+    case PT_PHDR:
+      if (*pdyn_bias == (GElf_Addr) - 1
+	  /* Do a sanity check on the putative address.  */
+	  && ((vaddr & (dwfl->segment_align - 1))
+	      == (phdr & (dwfl->segment_align - 1))))
+	{
+	  *pdyn_bias = phdr - vaddr;
+	  return *pdyn_vaddr != 0;
+	}
+      break;
+
+    case PT_DYNAMIC:
+      *pdyn_vaddr = vaddr;
+      *pdyn_filesz = filesz;
+      return *pdyn_bias != (GElf_Addr) - 1;
+    }
+
+  return false;
+}
+
+#define consider_phdr(type, vaddr, filesz) \
+  do_consider_phdr (type, vaddr, filesz, \
+                    dwfl, phdr, &dyn_bias, &dyn_vaddr, &dyn_filesz)
+
 int
 dwfl_link_map_report (Dwfl *dwfl, const void *auxv, size_t auxv_size,
 		      Dwfl_Memory_Callback *memory_callback,
@@ -750,31 +809,6 @@ dwfl_link_map_report (Dwfl *dwfl, const void *auxv, size_t auxv_size,
       GElf_Xword dyn_filesz = 0;
       GElf_Addr dyn_bias = (GElf_Addr) -1;
 
-      inline bool consider_phdr (GElf_Word type,
-				 GElf_Addr vaddr, GElf_Xword filesz)
-      {
-	switch (type)
-	  {
-	  case PT_PHDR:
-	    if (dyn_bias == (GElf_Addr) -1
-		/* Do a sanity check on the putative address.  */
-		&& ((vaddr & (dwfl->segment_align - 1))
-		    == (phdr & (dwfl->segment_align - 1))))
-	      {
-		dyn_bias = phdr - vaddr;
-		return dyn_vaddr != 0;
-	      }
-	    break;
-
-	  case PT_DYNAMIC:
-	    dyn_vaddr = vaddr;
-	    dyn_filesz = filesz;
-	    return dyn_bias != (GElf_Addr) -1;
-	  }
-
-	return false;
-      }
-
       if (phdr != 0 && phnum != 0)
 	{
 	  Dwfl_Module *phdr_mod;
-- 
2.6.0.rc2.230.g3dd15c0

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-01-06 19:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-03 21:25 [PATCH] Move nested functions in link_map.c to file scope Mark Wielaard
  -- strict thread matches above, loose matches on Subject: below --
2016-01-06 19:35 Chih-Hung Hsieh
2015-11-17 22:45 Chih-Hung Hsieh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).