From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============2242786328430298797==" MIME-Version: 1.0 From: Mark Wielaard To: elfutils-devel@lists.fedorahosted.org Subject: Re: Small code cleanup in dwfl_segment_report_module.c (Was: [commit] [PATCHv2 1/2] Add is_executable to Dwfl_Module.) Date: Tue, 23 Sep 2014 17:18:05 +0200 Message-ID: <1411485485.27706.46.camel@bordewijk.wildebeest.org> In-Reply-To: 20140923125858.GA22654@host2.jankratochvil.net --===============2242786328430298797== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable On Tue, 2014-09-23 at 14:58 +0200, Jan Kratochvil wrote: > By jumping on the 'e_ident' tag one could say what is being referenced. > But now 'buffer' is 'void *' and it is being cast to 'const unsigned char= *'. > And neither 'EI_DATA' nor 'EI_CLASS' reference 'e_ident' in their comment. Sure, that avoids one cast, which is always nice. Patch updated. Thanks, Mark commit f2f7bd73ef09da833ff5116aca6237b86cefc7df Author: Mark Wielaard 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 u= se them directly. = Signed-off-by: Mark Wielaard 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 + + * 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 = * 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_re= port_module.c index 3393b08..a4fbb08 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,10 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const= char *name, } = /* Extract the information we need from the file header. */ + const unsigned char *e_ident; + unsigned char ei_class; + unsigned char ei_data; + uint16_t e_type; union { Elf32_Ehdr e32; @@ -183,13 +187,16 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, cons= t char *name, .d_size =3D sizeof ehdr, .d_version =3D EV_CURRENT, }; - switch (((const unsigned char *) buffer)[EI_CLASS]) + e_ident =3D ((const unsigned char *) buffer); + ei_class =3D e_ident[EI_CLASS]; + ei_data =3D e_ident[EI_DATA]; + switch (ei_class) { case ELFCLASS32: xlatefrom.d_size =3D sizeof (Elf32_Ehdr); - if (elf32_xlatetom (&xlateto, &xlatefrom, - ((const unsigned char *) buffer)[EI_DATA]) =3D=3D NULL) + if (elf32_xlatetom (&xlateto, &xlatefrom, ei_data) =3D=3D NULL) return finish (); + e_type =3D ehdr.e32.e_type; phoff =3D ehdr.e32.e_phoff; phnum =3D ehdr.e32.e_phnum; phentsize =3D ehdr.e32.e_phentsize; @@ -200,9 +207,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const = char *name, = case ELFCLASS64: xlatefrom.d_size =3D sizeof (Elf64_Ehdr); - if (elf64_xlatetom (&xlateto, &xlatefrom, - ((const unsigned char *) buffer)[EI_DATA]) =3D=3D NULL) + if (elf64_xlatetom (&xlateto, &xlatefrom, ei_data) =3D=3D NULL) return finish (); + e_type =3D ehdr.e64.e_type; phoff =3D ehdr.e64.e_phoff; phnum =3D ehdr.e64.e_phnum; phentsize =3D ehdr.e64.e_phentsize; @@ -281,7 +288,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const = char *name, assert (sizeof (Elf32_Nhdr) =3D=3D sizeof (Elf64_Nhdr)); = void *notes; - if (ehdr.e32.e_ident[EI_DATA] =3D=3D MY_ELFDATA) + if (ei_data =3D=3D MY_ELFDATA) notes =3D data; else { @@ -397,10 +404,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const= char *name, break; } } - if (ehdr.e32.e_ident[EI_CLASS] =3D=3D ELFCLASS32) + if (ei_class =3D=3D ELFCLASS32) { - if (elf32_xlatetom (&xlateto, &xlatefrom, - ehdr.e32.e_ident[EI_DATA]) =3D=3D NULL) + if (elf32_xlatetom (&xlateto, &xlatefrom, ei_data) =3D=3D NULL) found_bias =3D false; /* Trigger error check. */ else for (uint_fast16_t i =3D 0; i < phnum; ++i) @@ -411,8 +417,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const = char *name, } else { - if (elf64_xlatetom (&xlateto, &xlatefrom, - ehdr.e32.e_ident[EI_DATA]) =3D=3D NULL) + if (elf64_xlatetom (&xlateto, &xlatefrom, ei_data) =3D=3D NULL) found_bias =3D false; /* Trigger error check. */ else for (uint_fast16_t i =3D 0; i < phnum; ++i) @@ -568,7 +573,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const = char *name, return soname_stroff !=3D 0 && dynstr_vaddr !=3D 0 && dynstrsz !=3D 0; } = - const size_t dyn_entsize =3D (ehdr.e32.e_ident[EI_CLASS] =3D=3D ELFCLASS= 32 + const size_t dyn_entsize =3D (ei_class =3D=3D ELFCLASS32 ? sizeof (Elf32_Dyn) : sizeof (Elf64_Dyn)); void *dyn_data =3D NULL; size_t dyn_data_size =3D 0; @@ -587,18 +592,16 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, cons= t char *name, xlateto.d_buf =3D &dyn; xlateto.d_size =3D sizeof dyn; = - if (ehdr.e32.e_ident[EI_CLASS] =3D=3D ELFCLASS32) + if (ei_class =3D=3D ELFCLASS32) { - if (elf32_xlatetom (&xlateto, &xlatefrom, - ehdr.e32.e_ident[EI_DATA]) !=3D NULL) + if (elf32_xlatetom (&xlateto, &xlatefrom, ei_data) !=3D NULL) for (size_t i =3D 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]) !=3D NULL) + if (elf64_xlatetom (&xlateto, &xlatefrom, ei_data) !=3D NULL) for (size_t i =3D 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 +611,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 =3D=3D NULL) - name =3D ehdr.e32.e_type =3D=3D ET_EXEC ? "[exe]" : execlike ? "[pie]"= : "[dso]"; + name =3D e_type =3D=3D ET_EXEC ? "[exe]" : execlike ? "[pie]" : "[dso]= "; = void *soname =3D NULL; size_t soname_size =3D 0; @@ -716,7 +719,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] =3D=3D ELFCLASS32) + if (ei_class =3D=3D ELFCLASS32) for (uint_fast16_t i =3D 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); --===============2242786328430298797==--