Hi Aaron, On Fri, Aug 02, 2024 at 07:38:02PM -0400, Aaron Merey wrote: > From: Heather McIntyre <h...@rice.edu> > > Add struct search_tree to hold tree root and lock. Add new eu_t* > functions for ensuring synchronized tree access. > > Replace tsearch, tfind, etc with eu_t* equivalents. > > lib: > * Makefile.am (libeu_a_SOURCES): Add eu-search.c. > (noinst_HEADERS): Add eu-search.h and locks.h. > * eu-config.h: Move rwlock macros to locks.h. > * eu-search.c: New file containing tree search functions with > locking. > * eu-search.h: New file. > * locks.h: New file containing rwlock macros previously in > eu-config.h. > libdw: > * cfi.h (struct Dwarf_CFI_s): Change type of search tree members > from void * to search_tree. > * cie.c: Replace tree search functions with eu-search equivalents. > * dwarf_begin_elf.c (valid_p): Initialize search trees. > * dwarf_end.c (cu_free): Replace tree search functions > with eu-search equivalents. > * dwarf_getcfi.c (dwarf_getcfi): Initialize search trees. > * dwarf_getlocations.c: Replace search tree functions with > eu-search equivalents. > (__libdw_intern_expression): Change type of cache parameter to > search_tree *. > * dwarf_getmacros.c: Replace tree search functions with > eu-search equivalents. > * dwarf_getsrclines.c: Ditto. > * fde.c: Ditto. > * frame-cache.c (__libdw_destroy_frame_cache): Initialize search > trees. > * libdwP.h (struct Dwarf): Change type of search tree members > from void * to search_tree. > (struct Dwarf_CU): Ditto. > (__libdw_intern_expression): Change type of cache parameter to > search_tree *. > * libdw_find_split_unit.c: Replace tree search functions with > eu-search equivalents. > * libdw_findcu.c: Ditto. > libdwfl: > * cu.c: Ditto. > * libdwflP.h (struct Dwfl_Module): Replace void *lazy_cu_root > with search_tree lazy_cu_tree. > libelf: > * elf_begin.c (file_read_elf): Initialize rawchunck_tree. > * elf_end.c (elf_end): Replace tree search function with > eu-search equivalent. > * elf_getdata_rawchunck.c: Ditto. > * libelfP.h (struct Elf): Replace void * rawchuncks memeber with > search_tree rawchunk_tree.
Nice Changelog. (note the memeber typo) > Signed-off-by: Heather S. McIntyre <h...@rice.edu> > Signed-off-by: Aaron Merey <ame...@redhat.com> > Signed-off-by: Mark Wielaard <m...@klomp.org> > > --- > > v3 changes: > > Moved most locking code to another patch in this series. > include "eu-search.h" and "locks.h" instead of <eu-search.h> > and <locks.h> Thanks, sorry for the extra "style" work. > On Fri, Jul 19, 2024 at 8:51 AM Mark Wielaard <m...@klomp.org> wrote: > > Is eu_tdestroy ever used now? > > It looks like everything now uses eu_search_tree_fine. > > Could still be a useful function to have, just checking. > > Yes the function less_lazy in libdwfl/cu.c uses it. Ack. I see I actually mentioned that myself below. Doh. > > > +#define LOCKS_H 1 > > > + > > > +#ifdef USE_LOCKS > > > +# include <pthread.h> > > > +# include <assert.h> > > > > Why the assert.h include? > > It's needed for assert_perror in the RWLOCK_CALL macro Interesting. That is existing code. So ok. > > > /* We know about all the CUs now, we don't need this table. */ > > > - tdestroy (mod->lazy_cu_root, nofree); > > > - mod->lazy_cu_root = NULL; > > > + eu_tdestroy (&mod->lazy_cu_tree, nofree); > > > } > > > > OK, a eu_tdestroy here, and then there is a eu_search_tree_fini in > > __libdwfl_module_free, which will call eu_tdestroy again (on the now > > NULL tree). Is that correct? Does [eu_]tdestroy handle NULL trees? > > Yes NULL is a valid (empty) tree. Good. And it looks like you also got rid of the extra dwarf,abbrev,split_locks. Looks good to go to me. Thanks, Mark
