On Wed, 10 Sep 2014 22:15:42 +0200, Mark Wielaard wrote:
> So I propose a cleanup like the attached first.

While I find that as an improvement in general IMO on top of your patch the
changes could be done a bit differently.

Patricularly I at least miss there that 'e_ident'.


Jan

diff --git a/libdwfl/dwfl_segment_report_module.c b/libdwfl/dwfl_segment_report_module.c
index 572f15b..be6950d 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -163,11 +163,11 @@ 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;
     Elf64_Ehdr e64;
+    Elf_Ehdr e;
   } ehdr;
   GElf_Off phoff;
   uint_fast16_t phnum;
@@ -186,15 +186,14 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
       .d_size = sizeof ehdr,
       .d_version = EV_CURRENT,
     };
-  ei_class = ((const unsigned char *) buffer)[EI_CLASS];
-  ei_data = ((const unsigned char *) buffer)[EI_DATA];
+  ei_class = ((const Elf_Ehdr *) buffer)->e_ident[EI_CLASS];
+  ei_data = ((const Elf_Ehdr *) buffer)->e_ident[EI_DATA];
   switch (ei_class)
     {
     case ELFCLASS32:
       xlatefrom.d_size = sizeof (Elf32_Ehdr);
       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;
@@ -207,7 +206,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
       xlatefrom.d_size = sizeof (Elf64_Ehdr);
       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;
@@ -609,7 +607,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 = e_type == ET_EXEC ? "[exe]" : execlike ? "[pie]" : "[dso]";
+    name = ehdr.e.e_type == ET_EXEC ? "[exe]" : execlike ? "[pie]" : "[dso]";
 
   void *soname = NULL;
   size_t soname_size = 0;
diff --git a/libelf/elf.h b/libelf/elf.h
index 40e87b2..47e7bc7 100644
--- a/libelf/elf.h
+++ b/libelf/elf.h
@@ -67,6 +67,14 @@ typedef Elf64_Half Elf64_Versym;
 typedef struct
 {
   unsigned char	e_ident[EI_NIDENT];	/* Magic number and other info */
+  uint16_t	e_type;			/* Object file type */
+  uint16_t	e_machine;		/* Architecture */
+  uint32_t	e_version;		/* Object file version */
+} Elf_Ehdr;
+
+typedef struct
+{
+  unsigned char	e_ident[EI_NIDENT];	/* Magic number and other info */
   Elf32_Half	e_type;			/* Object file type */
   Elf32_Half	e_machine;		/* Architecture */
   Elf32_Word	e_version;		/* Object file version */