* Remove nested functions from link_map.c V2
@ 2020-12-07 11:11 tbaeder
2020-12-07 11:11 ` [PATCH 1/2] link_map: Pull release_buffer() into file scope tbaeder
2020-12-07 11:11 ` [PATCH 2/2] link_map: Pull read_addrs() " tbaeder
0 siblings, 2 replies; 5+ messages in thread
From: tbaeder @ 2020-12-07 11:11 UTC (permalink / raw)
To: elfutils-devel
Same as the old thread. I used a memory_closure struct this time.
I quickly looked over the patches and I think we could even save
buffer and buffer_available in there to reduce the parameter count
further but then we should rename the struct (but read_data isn't the
best name either).
Hope the changelog entries are fine, I'll try to add them from now on.
Thanks,
Timm
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] link_map: Pull release_buffer() into file scope
2020-12-07 11:11 Remove nested functions from link_map.c V2 tbaeder
@ 2020-12-07 11:11 ` tbaeder
2020-12-10 13:00 ` Mark Wielaard
2020-12-07 11:11 ` [PATCH 2/2] link_map: Pull read_addrs() " tbaeder
1 sibling, 1 reply; 5+ messages in thread
From: tbaeder @ 2020-12-07 11:11 UTC (permalink / raw)
To: elfutils-devel
From: Timm Bäder <tbaeder@redhat.com>
Get rid of a nested function this way. Add a memory_closure struct to
keep the functions clean.
Signed-off-by: Timm Bäder <tbaeder@redhat.com>
---
libdwfl/ChangeLog | 5 +++++
libdwfl/link_map.c | 48 ++++++++++++++++++++++++++++++----------------
2 files changed, 37 insertions(+), 16 deletions(-)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index f11abb80..a5e6a4a7 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2020-12-07 Timm Bäder <tbaeder@redhat.com>
+
+ * link_map.c (report_r_debug): Pull release_buffer() function into
+ file scope. Add memory_closure struct.
+
2020-12-01 Timm Bäder <tbaeder@redhat.com>
* link_map.c (dwfl_link_map_report): Removed consider_phdr function
diff --git a/libdwfl/link_map.c b/libdwfl/link_map.c
index bcff8db5..f0119a97 100644
--- a/libdwfl/link_map.c
+++ b/libdwfl/link_map.c
@@ -225,6 +225,24 @@ addrsize (uint_fast8_t elfclass)
return elfclass * 4;
}
+struct memory_closure
+{
+ Dwfl *dwfl;
+ Dwfl_Memory_Callback *callback;
+ void *arg;
+};
+
+static inline int
+release_buffer (struct memory_closure *closure,
+ void **buffer, size_t *buffer_available, int result)
+{
+ if (*buffer != NULL)
+ (*closure->callback) (closure->dwfl, -1, buffer, buffer_available, 0, 0,
+ closure->arg);
+
+ return result;
+}
+
/* 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
@@ -249,15 +267,9 @@ 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];
+ struct memory_closure memory_closure = { dwfl, memory_callback,
+ memory_callback_arg };
inline bool read_addrs (GElf_Addr vaddr, size_t n)
{
size_t nb = n * addrsize (elfclass); /* Address words -> bytes to read. */
@@ -267,7 +279,7 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
|| vaddr < read_vaddr
|| vaddr - read_vaddr + nb > buffer_available)
{
- release_buffer (0);
+ release_buffer (&memory_closure, &buffer, &buffer_available, 0);
read_vaddr = vaddr;
int segndx = INTUSE(dwfl_addrsegment) (dwfl, vaddr, NULL);
@@ -304,7 +316,7 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
}
if (unlikely (read_addrs (read_vaddr, 1)))
- return release_buffer (-1);
+ return release_buffer (&memory_closure, &buffer, &buffer_available, -1);
GElf_Addr next = addrs[0];
@@ -319,7 +331,7 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
while (next != 0 && ++iterations < dwfl->lookup_elts)
{
if (read_addrs (next, 4))
- return release_buffer (-1);
+ return release_buffer (&memory_closure, &buffer, &buffer_available, 0);
/* Unused: l_addr is the difference between the address in memory
and the ELF file when the core was created. We need to
@@ -345,7 +357,7 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
name = l_name - read_vaddr + buffer;
else
{
- release_buffer (0);
+ release_buffer (&memory_closure, &buffer, &buffer_available, 0);
read_vaddr = l_name;
int segndx = INTUSE(dwfl_addrsegment) (dwfl, l_name, NULL);
if (likely (segndx >= 0)
@@ -372,7 +384,8 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
r_debug_info_module = malloc (sizeof (*r_debug_info_module)
+ strlen (name1) + 1);
if (unlikely (r_debug_info_module == NULL))
- return release_buffer (result);
+ release_buffer (&memory_closure, &buffer,
+ &buffer_available, result);
r_debug_info_module->fd = -1;
r_debug_info_module->elf = NULL;
r_debug_info_module->l_ld = l_ld;
@@ -413,7 +426,8 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
GElf_Addr build_id_vaddr = (build_id_elfaddr
- elf_dynamic_vaddr + l_ld);
- release_buffer (0);
+ release_buffer (&memory_closure, &buffer,
+ &buffer_available, 0);
int segndx = INTUSE(dwfl_addrsegment) (dwfl,
build_id_vaddr,
NULL);
@@ -432,7 +446,9 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
/* File has valid build-id which does not match
the one in memory. */
valid = false;
- release_buffer (0);
+ release_buffer (&memory_closure, &buffer,
+ &buffer_available, 0);
+
}
}
@@ -498,7 +514,7 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
}
}
- return release_buffer (result);
+ return release_buffer (&memory_closure, &buffer, &buffer_available, result);
}
\f
static GElf_Addr
--
2.26.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] link_map: Pull read_addrs() into file scope
2020-12-07 11:11 Remove nested functions from link_map.c V2 tbaeder
2020-12-07 11:11 ` [PATCH 1/2] link_map: Pull release_buffer() into file scope tbaeder
@ 2020-12-07 11:11 ` tbaeder
2020-12-10 13:14 ` Mark Wielaard
1 sibling, 1 reply; 5+ messages in thread
From: tbaeder @ 2020-12-07 11:11 UTC (permalink / raw)
To: elfutils-devel
From: Timm Bäder <tbaeder@redhat.com>
Get rid of a nested function this way.
Signed-off-by: Timm Bäder <tbaeder@redhat.com>
---
libdwfl/ChangeLog | 5 +++
libdwfl/link_map.c | 104 +++++++++++++++++++++++++--------------------
2 files changed, 62 insertions(+), 47 deletions(-)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index a5e6a4a7..d4e77f55 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2020-12-07 Timm Bäder <tbaeder@redhat.com>
+
+ * link_map.c (report_r_debug): Pull read_addrs() function into
+ file scope.
+
2020-12-07 Timm Bäder <tbaeder@redhat.com>
* link_map.c (report_r_debug): Pull release_buffer() function into
diff --git a/libdwfl/link_map.c b/libdwfl/link_map.c
index f0119a97..5a6a4dd9 100644
--- a/libdwfl/link_map.c
+++ b/libdwfl/link_map.c
@@ -243,6 +243,57 @@ release_buffer (struct memory_closure *closure,
return result;
}
+static inline bool
+read_addrs (struct memory_closure *closure,
+ uint_fast8_t elfclass, uint_fast8_t elfdata,
+ void **buffer, size_t *buffer_available,
+ GElf_Addr vaddr, GElf_Addr *read_vaddr,
+ size_t n, GElf_Addr *addrs /* [4] */)
+{
+ size_t nb = n * addrsize (elfclass); /* Address words -> bytes to read. */
+ Dwfl *dwfl = closure->dwfl;
+
+ /* 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 (closure, buffer, buffer_available, 0);
+
+ *read_vaddr = vaddr;
+ int segndx = INTUSE(dwfl_addrsegment) (dwfl, vaddr, NULL);
+ if (unlikely (segndx < 0)
+ || unlikely (! (*closure->callback) (dwfl, segndx,
+ buffer, buffer_available,
+ vaddr, nb, closure->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;
+}
+
/* 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
@@ -270,52 +321,9 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
GElf_Addr addrs[4];
struct memory_closure memory_closure = { dwfl, memory_callback,
memory_callback_arg };
- 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 (&memory_closure, &buffer, &buffer_available, 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)))
+ if (unlikely (read_addrs (&memory_closure, elfclass, elfdata,
+ &buffer, &buffer_available, read_vaddr, &read_vaddr,
+ 1, addrs)))
return release_buffer (&memory_closure, &buffer, &buffer_available, -1);
GElf_Addr next = addrs[0];
@@ -330,7 +338,9 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
size_t iterations = 0;
while (next != 0 && ++iterations < dwfl->lookup_elts)
{
- if (read_addrs (next, 4))
+ if (read_addrs (&memory_closure, elfclass, elfdata,
+ &buffer, &buffer_available, next, &read_vaddr,
+ 4, addrs))
return release_buffer (&memory_closure, &buffer, &buffer_available, 0);
/* Unused: l_addr is the difference between the address in memory
--
2.26.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] link_map: Pull release_buffer() into file scope
2020-12-07 11:11 ` [PATCH 1/2] link_map: Pull release_buffer() into file scope tbaeder
@ 2020-12-10 13:00 ` Mark Wielaard
0 siblings, 0 replies; 5+ messages in thread
From: Mark Wielaard @ 2020-12-10 13:00 UTC (permalink / raw)
To: tbaeder, elfutils-devel
Hi Timm,
On Mon, 2020-12-07 at 12:11 +0100, Timm Bäder via Elfutils-devel wrote:
> Get rid of a nested function this way. Add a memory_closure struct to
> keep the functions clean.
Pushed with one fix.
> @@ -319,7 +331,7 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata,
> while (next != 0 && ++iterations < dwfl->lookup_elts)
> {
> if (read_addrs (next, 4))
> - return release_buffer (-1);
> + return release_buffer (&memory_closure, &buffer, &buffer_available, 0);
The last argument must be -1.
Cheers,
Mark
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] link_map: Pull read_addrs() into file scope
2020-12-07 11:11 ` [PATCH 2/2] link_map: Pull read_addrs() " tbaeder
@ 2020-12-10 13:14 ` Mark Wielaard
0 siblings, 0 replies; 5+ messages in thread
From: Mark Wielaard @ 2020-12-10 13:14 UTC (permalink / raw)
To: tbaeder, elfutils-devel
Hi Timm,
On Mon, 2020-12-07 at 12:11 +0100, Timm Bäder via Elfutils-devel wrote:
> Get rid of a nested function this way.
The whole idea behind using nested functions is that we don't need to
pass around 9 arguments... But ok, it looks harmless. Pushed.
Cheers,
Mark
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-10 13:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-07 11:11 Remove nested functions from link_map.c V2 tbaeder
2020-12-07 11:11 ` [PATCH 1/2] link_map: Pull release_buffer() into file scope tbaeder
2020-12-10 13:00 ` Mark Wielaard
2020-12-07 11:11 ` [PATCH 2/2] link_map: Pull read_addrs() " tbaeder
2020-12-10 13:14 ` Mark Wielaard
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).