From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id E96233858C41 for ; Tue, 10 Oct 2023 21:25:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E96233858C41 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org Received: by gnu.wildebeest.org (Postfix, from userid 1000) id D6515300B302; Tue, 10 Oct 2023 23:25:33 +0200 (CEST) Date: Tue, 10 Oct 2023 23:25:33 +0200 From: Mark Wielaard To: elfutils-devel@sourceware.org Cc: hsm2@rice.edu Subject: Re: [PATCH 09/16] src: Use eu-search in nm and findtextrel. Message-ID: <20231010212533.GM728@gnu.wildebeest.org> References: <301fac87e83ebbbd677750579ae9a3429b461bdf.camel@klomp.org> <20231010134300.53830-1-mark@klomp.org> <20231010134300.53830-9-mark@klomp.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231010134300.53830-9-mark@klomp.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-3034.5 required=5.0 tests=BAYES_00,GIT_PATCH_0,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Heather, On Tue, Oct 10, 2023 at 03:42:53PM +0200, Mark Wielaard wrote: > From: Heather McIntyre > > * src/Makefile.am: Add USE_LOCKS condition for -pthread. > * src/findtextrel.c: Add eu-search.h and remove search.h. > Change calls of tsearch/tfind to eu_tsearch/eu_tfind. > * src/nm.c: Likewise. This does look technically correct. But both nm and findtextrel are single threaded programs. It might be interesting to try to make them parallel. But currently I think this would just introduce unnecessary locking. Cheers, Mark > Signed-off-by: Heather S. McIntyre > Signed-off-by: Mark Wielaard > --- > src/Makefile.am | 3 +++ > src/findtextrel.c | 10 +++++----- > src/nm.c | 10 +++++----- > 3 files changed, 13 insertions(+), 10 deletions(-) > > diff --git a/src/Makefile.am b/src/Makefile.am > index 10d59a48..fea5d43e 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -22,6 +22,9 @@ DEFS += $(YYDEBUG) -DDEBUGPRED=@DEBUGPRED@ \ > AM_CPPFLAGS += -I$(srcdir)/../libelf -I$(srcdir)/../libebl \ > -I$(srcdir)/../libdw -I$(srcdir)/../libdwelf \ > -I$(srcdir)/../libdwfl -I$(srcdir)/../libasm > +if USE_LOCKS > + AM_CFLAGS += -pthread > +endif > > AM_LDFLAGS = -Wl,-rpath-link,../libelf:../libdw $(STACK_USAGE_NO_ERROR) > > diff --git a/src/findtextrel.c b/src/findtextrel.c > index d3021a3a..5ac4c29a 100644 > --- a/src/findtextrel.c > +++ b/src/findtextrel.c > @@ -27,7 +27,7 @@ > #include > #include > #include > -#include > +#include > #include > #include > #include > @@ -501,10 +501,10 @@ check_rel (size_t nsegments, struct segments segments[nsegments], > /* There can be more than one relocation against one file. > Try to avoid multiple messages. And yes, the code uses > pointer comparison. */ > - if (tfind (src, knownsrcs, ptrcompare) == NULL) > + if (eu_tfind (src, knownsrcs, ptrcompare) == NULL) > { > printf (_("%s not compiled with -fpic/-fPIC\n"), src); > - tsearch (src, knownsrcs, ptrcompare); > + eu_tsearch (src, knownsrcs, ptrcompare); > } > return; > } > @@ -555,12 +555,12 @@ check_rel (size_t nsegments, struct segments segments[nsegments], > if (sym->st_value + sym->st_size > addr) > { > /* It is this function. */ > - if (tfind (lowstr, knownsrcs, ptrcompare) == NULL) > + if (eu_tfind (lowstr, knownsrcs, ptrcompare) == NULL) > { > printf (_("\ > the file containing the function '%s' is not compiled with -fpic/-fPIC\n"), > lowstr); > - tsearch (lowstr, knownsrcs, ptrcompare); > + eu_tsearch (lowstr, knownsrcs, ptrcompare); > } > } > else if (highidx == -1) > diff --git a/src/nm.c b/src/nm.c > index fbdee8e1..44c20fb2 100644 > --- a/src/nm.c > +++ b/src/nm.c > @@ -32,7 +32,7 @@ > #include > #include > #include > -#include > +#include > #include > #include > #include > @@ -537,7 +537,7 @@ static int > get_global (Dwarf *dbg __attribute__ ((unused)), Dwarf_Global *global, > void *arg __attribute__ ((unused))) > { > - tsearch (memcpy (xmalloc (sizeof (Dwarf_Global)), global, > + eu_tsearch (memcpy (xmalloc (sizeof (Dwarf_Global)), global, > sizeof (Dwarf_Global)), > &global_root, global_compare); > > @@ -696,7 +696,7 @@ get_local_names (Dwarf *dbg) > /* Check whether a similar local_name is already in the > cache. That should not happen. But if it does, we > don't want to leak memory. */ > - struct local_name **tres = tsearch (newp, &local_root, > + struct local_name **tres = eu_tsearch (newp, &local_root, > local_compare); > if (tres == NULL) > error_exit (errno, _("cannot create search tree")); > @@ -1387,7 +1387,7 @@ show_symbols (int fd, Ebl *ebl, GElf_Ehdr *ehdr, > && global_root != NULL) > { > Dwarf_Global fake = { .name = symstr }; > - Dwarf_Global **found = tfind (&fake, &global_root, > + Dwarf_Global **found = eu_tfind (&fake, &global_root, > global_compare); > if (found != NULL) > { > @@ -1442,7 +1442,7 @@ show_symbols (int fd, Ebl *ebl, GElf_Ehdr *ehdr, > .lowpc = sym->st_value, > .highpc = sym->st_value, > }; > - struct local_name **found = tfind (&fake, &local_root, > + struct local_name **found = eu_tfind (&fake, &local_root, > local_compare); > if (found != NULL) > { > -- > 2.41.0 >