Hi Aaron, On Fri, 2024-08-02 at 19:38 -0400, Aaron Merey wrote: > From: Heather McIntyre <h...@rice.edu> > > Add dwarf_lock for Dwarf as well as abbrev_lock for Dwarf_CU. > > * libdw/dwarf_begin_elf.c (dwarf_begin_elf): Init dwarf_lock. > * libdw/dwarf_end.c (cu_free): Free abbrev_lock. > (dwarf_end): Free dwarf_lock. > * libdw/libdwP.h (struct Dwarf): Define dwarf_lock. > (struct Dwarf_CU): Define abbrev_lock. > * libdw/libdw_findcu.c (__libdw_intern_next_unit): Init > abbrev_lock. > > Signed-off-by: Heather S. McIntyre <h...@rice.edu> > Signed-off-by: Aaron Merey <ame...@redhat.com> > > --- > v3 changes: > New patch added in v3 that contains rwlock changes previously in patch 3/9 v2.
The patch itself looks correct. But I think I would have introduced the locks separately and/or in the first patch that used them (dwarf_getalt for the dwarf_lock, dwarf_formref_die and __libdw_dieabbrev for abbrev_lock). It might also be a good idea to explicitly document which structure fields are "protected" by the separate locks to make reasoning about them a little easier. Cheers, Mark