Re: [PATCH] Support 1-sized reads in read_ubyte_unaligned_inc and read_sbyte_unaligned_inc

[Roland McGrath <roland@hack.frob.com>]

[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]

It sounds reasonable in the abstract.  It looks OK in the implementation,
except you should put parens around the macro arg when used that way.  

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.

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.

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?

The existing pair of uses is in a place where 2 is not even possible in
reality, though it looks like it is possible in theoretical malformed data.