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 7C7A13858C54 for ; Tue, 10 Oct 2023 16:51:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7C7A13858C54 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: from r6.localdomain (82-217-174-174.cable.dynamic.v4.ziggo.nl [82.217.174.174]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 2311E300B302; Tue, 10 Oct 2023 18:51:05 +0200 (CEST) Received: by r6.localdomain (Postfix, from userid 1000) id DD6823403D4; Tue, 10 Oct 2023 18:51:04 +0200 (CEST) Message-ID: <3d7ddb9ac7498f97388f3606dbda4115b6596d10.camel@klomp.org> Subject: Re: [PATCH 07/16] lib: Add eu_tsearch and eu_tfind From: Mark Wielaard To: elfutils-devel@sourceware.org Cc: hsm2@rice.edu Date: Tue, 10 Oct 2023 18:51:04 +0200 In-Reply-To: <20231010134300.53830-7-mark@klomp.org> References: <301fac87e83ebbbd677750579ae9a3429b461bdf.camel@klomp.org> <20231010134300.53830-1-mark@klomp.org> <20231010134300.53830-7-mark@klomp.org> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) MIME-Version: 1.0 X-Spam-Status: No, score=-3033.6 required=5.0 tests=BAYES_00,GIT_PATCH_0,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,KAM_SHORT,RCVD_IN_BARRACUDACENTRAL,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, 2023-10-10 at 15:42 +0200, Mark Wielaard wrote: > From: Heather McIntyre >=20 > * lib/eu-search.h: New file. > Declarations for read/write locked eu_tsearch/eu_tfind. Like in the previous case, don't forget to add such a new header to noinst_HEADERS so make dist adds them. diff --git a/lib/Makefile.am b/lib/Makefile.am index ce8f3e1b..9df0a8c6 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -39,5 +39,6 @@ libeu_a_SOURCES =3D xasprintf.c xstrdup.c xstrndup.c xmal= loc.c next_prime.c \ =20 noinst_HEADERS =3D fixedsizehash.h libeu.h system.h dynamicsizehash.h list= .h \ eu-config.h color.h printversion.h bpf.h \ - atomics.h stdatomic-fbsd.h dynamicsizehash_concurrent.h + atomics.h stdatomic-fbsd.h dynamicsizehash_concurrent.h \ + eu-search.h EXTRA_DIST =3D dynamicsizehash.c dynamicsizehash_concurrent.c > * lib/eu-search.c: New file. > Definitions for read/write locked eu_tsearch/eu_tfind. > * Makefile.am (libeu_a_SOURCES): Add eu-search.c. >=20 > Signed-off-by: Heather S. McIntyre > Signed-off-by: Mark Wielaard > --- > lib/Makefile.am | 2 +- > lib/eu-search.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++ > lib/eu-search.h | 39 ++++++++++++++++++++++++++++++++ > 3 files changed, 100 insertions(+), 1 deletion(-) > create mode 100644 lib/eu-search.c > create mode 100644 lib/eu-search.h >=20 > diff --git a/lib/Makefile.am b/lib/Makefile.am > index b3bb929f..ce8f3e1b 100644 > --- a/lib/Makefile.am > +++ b/lib/Makefile.am > @@ -34,7 +34,7 @@ AM_CPPFLAGS +=3D -I$(srcdir)/../libelf > noinst_LIBRARIES =3D libeu.a > =20 > libeu_a_SOURCES =3D xasprintf.c xstrdup.c xstrndup.c xmalloc.c next_prim= e.c \ > - crc32.c crc32_file.c \ > + crc32.c crc32_file.c eu-search.c \ > color.c error.c printversion.c OK. > noinst_HEADERS =3D fixedsizehash.h libeu.h system.h dynamicsizehash.h li= st.h \ > diff --git a/lib/eu-search.c b/lib/eu-search.c > new file mode 100644 > index 00000000..a6b04f4f > --- /dev/null > +++ b/lib/eu-search.c > @@ -0,0 +1,60 @@ > +/* Definitions for thread-safe tsearch/tfind > + Copyright (C) 2023 Rice University > + This file is part of elfutils. > + > + This file is free software; you can redistribute it and/or modify > + it under the terms of either > + > + * the GNU Lesser General Public License as published by the Free > + Software Foundation; either version 3 of the License, or (at > + your option) any later version > + > + or > + > + * the GNU General Public License as published by the Free > + Software Foundation; either version 2 of the License, or (at > + your option) any later version > + > + or both in parallel, as here. > + > + elfutils is distributed in the hope that it will be useful, but > + WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + General Public License for more details. > + > + You should have received copies of the GNU General Public License and > + the GNU Lesser General Public License along with this program. If > + not, see . */ > + > +#ifdef HAVE_CONFIG_H > +#include > +#endif > + > +#include > +#include "eu-search.h" > + > +rwlock_define(static, search_find_lock); > + > +void *eu_tsearch(const void *key, void **rootp, > + int (*compar)(const void *, const void *)) > +{ > + void *ret =3D NULL; > + rwlock_wrlock(search_find_lock); > + > + ret =3D tsearch(key, rootp, compar); > + > + rwlock_unlock(search_find_lock); > + return ret; > +} > + > +void *eu_tfind(const void *key, void *const *rootp, > + int (*compar)(const void *, const void *)) > +{ > + void *ret =3D NULL; > + rwlock_rdlock(search_find_lock); > + > + ret =3D tfind(key, rootp, compar); > + > + rwlock_unlock(search_find_lock); > + return ret; > +} So this uses on static lock for all eu-tsearch/eu-tfind calls. But searches with different roots should be independent. Would it make sense to add different locks for different roots? That would make the code a little more complicated, but we could hide the root and lock in a new structure that will be passed the the eu_tsearch/eu_tfind calls. Maybe something like: struct eu_search_root { void *root; rwlock_define (,lock); }; With helper functions to create/init and destroy/delete them? void eu_tinit (struct eu_search_root *search_root); (Or is there no real initing to do?) void eu_tdestroy (struct eu_search_root *search_root, void (*free_node)(void *nodep)); Or is all this way too complicated and a global lock just fine? Cheers, Mark > diff --git a/lib/eu-search.h b/lib/eu-search.h > new file mode 100644 > index 00000000..4ce0139a > --- /dev/null > +++ b/lib/eu-search.h > @@ -0,0 +1,39 @@ > +/* Calls for thread-safe tsearch/tfind > + Copyright (C) 2023 Rice University > + This file is part of elfutils. > + > + This file is free software; you can redistribute it and/or modify > + it under the terms of either > + > + * the GNU Lesser General Public License as published by the Free > + Software Foundation; either version 3 of the License, or (at > + your option) any later version > + > + or > + > + * the GNU General Public License as published by the Free > + Software Foundation; either version 2 of the License, or (at > + your option) any later version > + > + or both in parallel, as here. > + > + elfutils is distributed in the hope that it will be useful, but > + WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + General Public License for more details. > + > + You should have received copies of the GNU General Public License and > + the GNU Lesser General Public License along with this program. If > + not, see . */ > + > +#ifndef EU_SEARCH_H > +#define EU_SEARCH_H 1 > + > +#include > + > +extern void *eu_tsearch(const void *key, void **rootp, > + int (*compar)(const void *, const void *)); > +extern void *eu_tfind(const void *key, void *const *rootp, > + int (*compar)(const void *, const void *)); > + > +#endif