On Fri, 2021-01-08 at 09:16 +0100, Timm Bäder via Elfutils-devel wrote: > here another round for src/readelf.c. I think they are simple, but I'm > not happy with the advance_pc() commit. I'm open for suggestions here > but I can't come up with something better right now. I'll keep looking > in to that in the meantime.
Yeah, the advance_pc function is really why you want nested functions in the first place IMHO. Passing around the captured state as 6 extra arguments seems to not really make the code much clearer. Especially because on its own it isn't immediately clear what the code is about or that it is to be used as part of the special opcode or DW_LNS_advance_pc opcode (where DW_LNS_const_add_pc acts like special opcode 255) handling. It might be helpful to spell out in a comment to the function which part of the DWARF4 6.2.5.1 Special Opcodes handling the advance_pc function is responsible for. But honestly in this case I think just keeping the nested function is the cleanest way to handle this. Cheers, Mark