Hi Timm, On Tue, 2021-02-09 at 12:45 +0100, Timm Bäder via Elfutils-devel wrote: > There is quite a bit of code duplication between src/readelf.c and > libdw/dwarf_getsrclines.c when parsing header_length, unit_length, > etc. > > Pull the code out into libdwP.h and use it in both of those files. > > It doesn't save a much code as I'd hoped and I'm not sure about the > naming, but here's the patch.
BTW. In these cases I really like having a ChangeLog entry to guide review. I think the struct should be named line_header_state. And the functions __libdw_line_header_state_parse and __libdw_line_header_get_increment to make clear they are internal __libdw functions. I think it would be nicer if __libdw_line_header_state_parse returned a bool (true on success, false on failure) and set any error itself using __libdw_seterrno when returning false. This might give read_srclines a more specific error and the readelf.c code can then add dwarf_errmsg (- 1) to its error message. This changes the readelf case a bit. It used to print the header even if some of the fields (version, address_size, segment_selector_size) weren't supported. It might be good if __libdw_line_header_state_parse would just set the values in the line_state_header and let the caller decide what to do with them. Cheers, Mark
