This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
Small code cleanup in dwfl_segment_report_module.c (Was: [commit] [PATCHv2 1/2] Add is_executable to Dwfl_Module.)
- From: Mark Wielaard <mjw at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Tue, 23 Sep 2014 14:50:13 +0200
- Subject: Small code cleanup in dwfl_segment_report_module.c (Was: [commit] [PATCHv2 1/2] Add is_executable to Dwfl_Module.)
On Thu, 2014-09-18 at 18:23 +0200, Jan Kratochvil wrote:
> On Wed, 10 Sep 2014 22:33:50 +0200, Mark Wielaard wrote:
> > On Wed, 2014-09-10 at 21:22 +0200, Jan Kratochvil wrote:
> > > libdwfl/
> > > 2014-09-10 Jan Kratochvil <jan.kratochvil@redhat.com>
> > >
> > > * dwfl_build_id_find_elf.c (dwfl_build_id_find_elf): Use IS_EXECUTABLE.
> > > * dwfl_segment_report_module.c (dwfl_segment_report_module): Set
> > > IS_EXECUTABLE.
> > > * libdwflP.h (struct Dwfl_Module): New field is_executable.
> >
> > I like this cleanup (modulo the already existing e32/e64 confusion in the code).
>
> Checked in as:
> 6097c00a539873e9baa22e10f9387b9c36c4fa25
>
> In the form as posted, without any cleanups being discussed.
Thanks. Updated cleanup based on current master attached.
I didn't take the new union member since I think that introduces the
same kind of confusion of which union member to use that we are trying
to fix in the first place.
Cheers,
Mark
commit c3fd359171dc2af4d034df4a8e511a1737702e0e
Author: Mark Wielaard <mjw@redhat.com>
Date: Tue Sep 23 14:48:38 2014 +0200
libdwfl: dwfl_segment_report_module use ei_class, ei_data and e_type.
To make it easier to see that the code is using the correct fields of
the ehdr e32/e64 union extract ei_class, ei_data and e_type early and use
them directly.
Signed-off-by: Mark Wielaard <mjw@redhat.com>
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index bfbc1f7..12bfa21 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2014-09-23 Mark Wielaard <mjw@redhat.com>
+
+ * dwfl_segment_report_module.c (dwfl_segment_report_module):
+ Extract ei_class, ei_data and e_type early and use the result.
+
2014-09-18 Jan Kratochvil <jan.kratochvil@redhat.com>
* dwfl_build_id_find_elf.c (dwfl_build_id_find_elf): Use IS_EXECUTABLE.
diff --git a/libdwfl/dwfl_segment_report_module.c b/libdwfl/dwfl_segment_report_module.c
index 3393b08..6441aed 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -1,5 +1,5 @@
/* Sniff out modules from ELF headers visible in memory segments.
- Copyright (C) 2008-2012 Red Hat, Inc.
+ Copyright (C) 2008-2012, 2014 Red Hat, Inc.
This file is part of elfutils.
This file is free software; you can redistribute it and/or modify
@@ -161,6 +161,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
}
/* Extract the information we need from the file header. */
+ unsigned char ei_class;
+ unsigned char ei_data;
+ uint16_t e_type;
union
{
Elf32_Ehdr e32;
@@ -183,13 +186,15 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
.d_size = sizeof ehdr,
.d_version = EV_CURRENT,
};
- switch (((const unsigned char *) buffer)[EI_CLASS])
+ ei_class = ((const unsigned char *) buffer)[EI_CLASS];
+ ei_data = ((const unsigned char *) buffer)[EI_DATA];
+ switch (ei_class)
{
case ELFCLASS32:
xlatefrom.d_size = sizeof (Elf32_Ehdr);
- if (elf32_xlatetom (&xlateto, &xlatefrom,
- ((const unsigned char *) buffer)[EI_DATA]) == NULL)
+ if (elf32_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL)
return finish ();
+ e_type = ehdr.e32.e_type;
phoff = ehdr.e32.e_phoff;
phnum = ehdr.e32.e_phnum;
phentsize = ehdr.e32.e_phentsize;
@@ -200,9 +205,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
case ELFCLASS64:
xlatefrom.d_size = sizeof (Elf64_Ehdr);
- if (elf64_xlatetom (&xlateto, &xlatefrom,
- ((const unsigned char *) buffer)[EI_DATA]) == NULL)
+ if (elf64_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL)
return finish ();
+ e_type = ehdr.e64.e_type;
phoff = ehdr.e64.e_phoff;
phnum = ehdr.e64.e_phnum;
phentsize = ehdr.e64.e_phentsize;
@@ -281,7 +286,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
assert (sizeof (Elf32_Nhdr) == sizeof (Elf64_Nhdr));
void *notes;
- if (ehdr.e32.e_ident[EI_DATA] == MY_ELFDATA)
+ if (ei_data == MY_ELFDATA)
notes = data;
else
{
@@ -397,10 +402,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
break;
}
}
- if (ehdr.e32.e_ident[EI_CLASS] == ELFCLASS32)
+ if (ei_class == ELFCLASS32)
{
- if (elf32_xlatetom (&xlateto, &xlatefrom,
- ehdr.e32.e_ident[EI_DATA]) == NULL)
+ if (elf32_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL)
found_bias = false; /* Trigger error check. */
else
for (uint_fast16_t i = 0; i < phnum; ++i)
@@ -411,8 +415,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
}
else
{
- if (elf64_xlatetom (&xlateto, &xlatefrom,
- ehdr.e32.e_ident[EI_DATA]) == NULL)
+ if (elf64_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL)
found_bias = false; /* Trigger error check. */
else
for (uint_fast16_t i = 0; i < phnum; ++i)
@@ -568,7 +571,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
return soname_stroff != 0 && dynstr_vaddr != 0 && dynstrsz != 0;
}
- const size_t dyn_entsize = (ehdr.e32.e_ident[EI_CLASS] == ELFCLASS32
+ const size_t dyn_entsize = (ei_class == ELFCLASS32
? sizeof (Elf32_Dyn) : sizeof (Elf64_Dyn));
void *dyn_data = NULL;
size_t dyn_data_size = 0;
@@ -587,18 +590,16 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
xlateto.d_buf = &dyn;
xlateto.d_size = sizeof dyn;
- if (ehdr.e32.e_ident[EI_CLASS] == ELFCLASS32)
+ if (ei_class == ELFCLASS32)
{
- if (elf32_xlatetom (&xlateto, &xlatefrom,
- ehdr.e32.e_ident[EI_DATA]) != NULL)
+ if (elf32_xlatetom (&xlateto, &xlatefrom, ei_data) != NULL)
for (size_t i = 0; i < dyn_filesz / sizeof dyn.d32[0]; ++i)
if (consider_dyn (dyn.d32[i].d_tag, dyn.d32[i].d_un.d_val))
break;
}
else
{
- if (elf64_xlatetom (&xlateto, &xlatefrom,
- ehdr.e32.e_ident[EI_DATA]) != NULL)
+ if (elf64_xlatetom (&xlateto, &xlatefrom, ei_data) != NULL)
for (size_t i = 0; i < dyn_filesz / sizeof dyn.d64[0]; ++i)
if (consider_dyn (dyn.d64[i].d_tag, dyn.d64[i].d_un.d_val))
break;
@@ -608,7 +609,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
/* We'll use the name passed in or a stupid default if not DT_SONAME. */
if (name == NULL)
- name = ehdr.e32.e_type == ET_EXEC ? "[exe]" : execlike ? "[pie]" : "[dso]";
+ name = e_type == ET_EXEC ? "[exe]" : execlike ? "[pie]" : "[dso]";
void *soname = NULL;
size_t soname_size = 0;
@@ -716,7 +717,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
final_read (offset, vaddr + bias, filesz);
}
- if (ehdr.e32.e_ident[EI_CLASS] == ELFCLASS32)
+ if (ei_class == ELFCLASS32)
for (uint_fast16_t i = 0; i < phnum; ++i)
read_phdr (phdrs.p32[i].p_type, phdrs.p32[i].p_vaddr,
phdrs.p32[i].p_offset, phdrs.p32[i].p_filesz);