Roland McGrath <roland(a)hack.frob.com> 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.
Thanks,
PM