This is the mail archive of the
mailing list for the elfutils project.
Re: [PATCH] Support 1-sized reads in read_ubyte_unaligned_inc and read_sbyte_unaligned_inc
- From: Petr Machata <pmachata at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Thu, 11 Sep 2014 01:46:10 +0200
- Subject: Re: [PATCH] Support 1-sized reads in read_ubyte_unaligned_inc and read_sbyte_unaligned_inc
Roland McGrath <firstname.lastname@example.org> writes:
> But, in actual fact, there are zero uses of read_sbyte_unaligned_inc and
> two uses of read_ubyte_unaligned_inc (they are adjacent in
> readelf.c:print_debug_frame_section). In both of those uses the size
> argument is a non-constant, so adding another case has runtime cost.
> Clearly that cost doesn't actually matter much in readelf, but it calls
> into question the wisdom of expanding these to more uses without
> considering their performance in those uses.
The performance implications did cross my mind, and I decided that it's
fine in readelf. (I did check the uses.)
> It also seems questionable from a source-comprehensibility perspective to
> make those support a different set of sizes than read_sbyte_unaligned and
> read_ubyte_unaligned do. But those have no uses at all, so perhaps they
> should just be removed anyway.
Actually I didn't notice the two other macros. I agree that such
inconsistency is bad.
That read_sbyte_unaligned_inc should be dropped did actually come to my
mind. Clearly it's untested and untestable as things currently stand.
> How many new uses are you adding, and what do they look like? I guess they
> must have non-constant size parameters too, or you would have just used the
> size-specific macros. So where does the size parameter come from that it
> can have any of the four possible sizes?
Five, four of them with constant size and one with a 64bit?8:4 sort of
expression. The reads are done through a macro that checks bounds,
There's one macro for all the widths, mostly because I didn't like to
have four macros with unknown cut'n'paste errors. I expect that the
compiler will be able to see through and just inline a check and an
access for the right width directly, but I didn't actually check.
Admittedly this is all somewhat moot. I don't check bounds with LEB's
anyway, and most of libdw just checks post fact that the pointers are
still in bounds. Maybe I should simply do the same.