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

Reply via email to