From 222470db604704ed128b3a54f2c54d47e2809d28 Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Mon, 9 Sep 2024 11:11:47 +0300 Subject: [PATCH 01/10] Take advantage of C++ native mutex facilities for macro locking C++ guarantees initialization of static objects takes place before threads can run, so we don't need all the locksInitialized fubar. The native objects also release themselves which fixes a theoretical leak we had: we never destroyed the lock or lockattr so they remained reachable until program exit. We could continue to acquire and release the lock in the corresponding functions but that's not RAII and would be inconsistent with what we do elsewhere in rpm, just convert to the local lock variable style. --- rpmio/macro.c | 86 +++++++++++++++++++-------------------------------- 1 file changed, 31 insertions(+), 55 deletions(-) diff --git a/rpmio/macro.c b/rpmio/macro.c index 3f34f718ea..4ae0906b64 100644 --- a/rpmio/macro.c +++ b/rpmio/macro.c @@ -5,12 +5,12 @@ #include "system.h" #include +#include #include #include #include #include -#include #include #include #ifdef HAVE_SCHED_GETAFFINITY @@ -66,44 +66,27 @@ struct rpmMacroEntry_s { }; using macroTable = std::map>; +using wrlock = std::lock_guard; /*! The structure used to store the set of macros in a context. */ -struct rpmMacroContext_s { - macroTable tab; /*!< Map of macro entry stacks */ - int depth; /*!< Depth tracking on external recursion */ - int level; /*!< Scope level tracking when on external recursion */ - pthread_mutex_t lock; - pthread_mutexattr_t lockattr; -}; - -static struct rpmMacroContext_s rpmGlobalMacroContext_s; -rpmMacroContext rpmGlobalMacroContext = &rpmGlobalMacroContext_s; - -static struct rpmMacroContext_s rpmCLIMacroContext_s; -rpmMacroContext rpmCLIMacroContext = &rpmCLIMacroContext_s; - /* * The macro engine internals do not require recursive mutexes but Lua * macro bindings which can get called from the internals use the external * interfaces which do perform locking. Until that is fixed somehow * we'll just have to settle for recursive mutexes. - * Unfortunately POSIX doesn't specify static initializers for recursive - * mutexes so we need to have a separate PTHREAD_ONCE initializer just - * to initialize the otherwise static macro context mutexes. Pooh. */ -static pthread_once_t locksInitialized = PTHREAD_ONCE_INIT; +struct rpmMacroContext_s { + macroTable tab {}; /*!< Map of macro entry stacks */ + int depth {}; /*!< Depth tracking on external recursion */ + int level {}; /*!< Scope level tracking when on external recursion */ + std::recursive_mutex mutex {}; +}; -static void initLocks(void) -{ - rpmMacroContext mcs[] = { rpmGlobalMacroContext, rpmCLIMacroContext, NULL }; +static struct rpmMacroContext_s rpmGlobalMacroContext_s; +rpmMacroContext rpmGlobalMacroContext = &rpmGlobalMacroContext_s; - for (rpmMacroContext *mcp = mcs; *mcp; mcp++) { - rpmMacroContext mc = *mcp; - pthread_mutexattr_init(&mc->lockattr); - pthread_mutexattr_settype(&mc->lockattr, PTHREAD_MUTEX_RECURSIVE); - pthread_mutex_init(&mc->lock, &mc->lockattr); - } -} +static struct rpmMacroContext_s rpmCLIMacroContext_s; +rpmMacroContext rpmCLIMacroContext = &rpmCLIMacroContext_s; /** * Macro expansion state. @@ -157,17 +140,9 @@ static rpmMacroContext rpmmctxAcquire(rpmMacroContext mc) { if (mc == NULL) mc = rpmGlobalMacroContext; - pthread_once(&locksInitialized, initLocks); - pthread_mutex_lock(&mc->lock); return mc; } -static rpmMacroContext rpmmctxRelease(rpmMacroContext mc) -{ - pthread_mutex_unlock(&mc->lock); - return NULL; -} - /** * Find entry in macro table. * @param mc macro context @@ -1911,8 +1886,9 @@ int rpmExpandMacros(rpmMacroContext mc, const char * sbuf, char ** obuf, int fla int rc; mc = rpmmctxAcquire(mc); + wrlock lock(mc->mutex); + rc = doExpandMacros(mc, sbuf, flags, &target); - rpmmctxRelease(mc); if (rc) { free(target); @@ -1930,6 +1906,8 @@ int rpmExpandThisMacro(rpmMacroContext mc, const char *n, ARGV_const_t args, ch int rc = 1; /* assume failure */ mc = rpmmctxAcquire(mc); + wrlock lock(mc->mutex); + mep = findEntry(mc, n, 0, NULL); if (mep) { rpmMacroBuf mb = mbCreate(mc, flags); @@ -1937,7 +1915,6 @@ int rpmExpandThisMacro(rpmMacroContext mc, const char *n, ARGV_const_t args, ch target = xstrdup(mb->buf.c_str()); delete mb; } - rpmmctxRelease(mc); if (rc) { free(target); return -1; @@ -1951,6 +1928,7 @@ void rpmDumpMacroTable(rpmMacroContext mc, FILE * fp) { mc = rpmmctxAcquire(mc); + wrlock lock(mc->mutex); if (fp == NULL) fp = stderr; fprintf(fp, "========================\n"); @@ -1966,7 +1944,6 @@ rpmDumpMacroTable(rpmMacroContext mc, FILE * fp) } fprintf(fp, _("======================== active %zu empty %d\n"), mc->tab.size(), 0); - rpmmctxRelease(mc); } int rpmPushMacroFlags(rpmMacroContext mc, @@ -1974,8 +1951,8 @@ int rpmPushMacroFlags(rpmMacroContext mc, int level, rpmMacroFlags flags) { mc = rpmmctxAcquire(mc); + wrlock lock(mc->mutex); pushMacro(mc, n, o, b, level, flags & RPMMACRO_LITERAL ? ME_LITERAL : ME_NONE); - rpmmctxRelease(mc); return 0; } @@ -1991,17 +1968,17 @@ int rpmPushMacroAux(rpmMacroContext mc, int level, rpmMacroFlags flags) { mc = rpmmctxAcquire(mc); + wrlock lock(mc->mutex); pushMacroAny(mc, n, nargs ? "" : NULL, "", f, priv, nargs, level, flags|ME_FUNC); - rpmmctxRelease(mc); return 0; } int rpmPopMacro(rpmMacroContext mc, const char * n) { mc = rpmmctxAcquire(mc); + wrlock lock(mc->mutex); popMacro(mc, n); - rpmmctxRelease(mc); return 0; } @@ -2010,8 +1987,8 @@ rpmDefineMacro(rpmMacroContext mc, const char * macro, int level) { int rc; mc = rpmmctxAcquire(mc); + wrlock lock(mc->mutex); rc = defineMacro(mc, macro, level); - rpmmctxRelease(mc); return rc; } @@ -2019,9 +1996,9 @@ int rpmMacroIsDefined(rpmMacroContext mc, const char *n) { int defined = 0; if ((mc = rpmmctxAcquire(mc)) != NULL) { + wrlock lock(mc->mutex); if (findEntry(mc, n, 0, NULL)) defined = 1; - rpmmctxRelease(mc); } return defined; } @@ -2030,10 +2007,10 @@ int rpmMacroIsParametric(rpmMacroContext mc, const char *n) { int parametric = 0; if ((mc = rpmmctxAcquire(mc)) != NULL) { + wrlock lock(mc->mutex); rpmMacroEntry mep = findEntry(mc, n, 0, NULL); if (mep && mep->opts) parametric = 1; - rpmmctxRelease(mc); } return parametric; } @@ -2052,11 +2029,10 @@ rpmLoadMacros(rpmMacroContext mc, int level) gmc = rpmmctxAcquire(NULL); mc = rpmmctxAcquire(mc); + wrlock glock(gmc->mutex); + wrlock lock(mc->mutex); copyMacros(mc, gmc, level); - - rpmmctxRelease(mc); - rpmmctxRelease(gmc); } int @@ -2065,8 +2041,8 @@ rpmLoadMacroFile(rpmMacroContext mc, const char * fn) int rc; mc = rpmmctxAcquire(mc); + wrlock lock(mc->mutex); rc = loadMacroFile(mc, fn); - rpmmctxRelease(mc); return rc; } @@ -2077,6 +2053,7 @@ rpmInitMacros(rpmMacroContext mc, const char * macrofiles) ARGV_t pattern, globs = NULL; rpmMacroContext climc; mc = rpmmctxAcquire(mc); + wrlock lock(mc->mutex); /* Define built-in macros */ for (const struct builtins_s *b = builtinmacros; b->name; b++) { @@ -2108,18 +2085,16 @@ rpmInitMacros(rpmMacroContext mc, const char * macrofiles) /* Reload cmdline macros */ climc = rpmmctxAcquire(rpmCLIMacroContext); + wrlock clilock(climc->mutex); copyMacros(climc, mc, RMIL_CMDLINE); - rpmmctxRelease(climc); - - rpmmctxRelease(mc); } void rpmFreeMacros(rpmMacroContext mc) { mc = rpmmctxAcquire(mc); + wrlock lock(mc->mutex); freeMacros(mc); - rpmmctxRelease(mc); } char * @@ -2138,8 +2113,9 @@ rpmExpand(const char *arg, ...) va_end(ap); rpmMacroContext mc = rpmmctxAcquire(NULL); + wrlock lock(mc->mutex); + (void) doExpandMacros(mc, buf.c_str(), 0, &ret); - rpmmctxRelease(mc); return ret; } From af066ad0aab46baa1f27d6c1c88126ece7babc92 Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Tue, 10 Sep 2024 17:18:30 +0300 Subject: [PATCH 02/10] Take advantage of C++ native mutex facilities for string pool Shared and exclusive locks are of different types in STL so we can't easily return one or the other from poolLock() as per the write argument. Just convert all poolLock() calls sites to name their lock type locally. --- rpmio/rpmstrpool.c | 71 ++++++++++++++++++++-------------------------- 1 file changed, 30 insertions(+), 41 deletions(-) diff --git a/rpmio/rpmstrpool.c b/rpmio/rpmstrpool.c index 5259aa0ef9..8044f933e3 100644 --- a/rpmio/rpmstrpool.c +++ b/rpmio/rpmstrpool.c @@ -1,7 +1,10 @@ #include "system.h" + +#include +#include + #include #include -#include #include #include #include "debug.h" @@ -41,23 +44,13 @@ struct rpmstrPool_s { poolHash hash; /* string -> sid hash table */ int frozen; /* are new id additions allowed? */ int nrefs; /* refcount */ - pthread_rwlock_t lock; + std::shared_mutex mutex; }; static inline const char *id2str(rpmstrPool pool, rpmsid sid); -static inline void poolLock(rpmstrPool pool, int write) -{ - if (write) - pthread_rwlock_wrlock(&pool->lock); - else - pthread_rwlock_rdlock(&pool->lock); -} - -static inline void poolUnlock(rpmstrPool pool) -{ - pthread_rwlock_unlock(&pool->lock); -} +using wrlock = std::unique_lock; +using rdlock = std::shared_lock; /* calculate hash and string length on at once */ static inline unsigned int rstrlenhash(const char * str, size_t * len) @@ -258,17 +251,15 @@ rpmstrPool rpmstrPoolCreate(void) rpmstrPoolRehash(pool); pool->nrefs = 1; - pthread_rwlock_init(&pool->lock, NULL); return pool; } rpmstrPool rpmstrPoolFree(rpmstrPool pool) { if (pool) { - poolLock(pool, 1); + wrlock lock(pool->mutex); if (pool->nrefs > 1) { pool->nrefs--; - poolUnlock(pool); } else { if (pool_debug) poolHashPrintStats(pool); @@ -278,8 +269,6 @@ rpmstrPool rpmstrPoolFree(rpmstrPool pool) pool->chunks[i] = _free(pool->chunks[i]); } free(pool->chunks); - poolUnlock(pool); - pthread_rwlock_destroy(&pool->lock); free(pool); } } @@ -289,9 +278,8 @@ rpmstrPool rpmstrPoolFree(rpmstrPool pool) rpmstrPool rpmstrPoolLink(rpmstrPool pool) { if (pool) { - poolLock(pool, 1); + wrlock lock(pool->mutex); pool->nrefs++; - poolUnlock(pool); } return pool; } @@ -301,7 +289,7 @@ void rpmstrPoolFreeze(rpmstrPool pool, int keephash) if (!pool) return; - poolLock(pool, 1); + wrlock lock(pool->mutex); if (!pool->frozen) { if (!keephash) { pool->hash = poolHashFree(pool->hash); @@ -311,18 +299,16 @@ void rpmstrPoolFreeze(rpmstrPool pool, int keephash) pool->offs_alloced * sizeof(*pool->offs)); pool->frozen = 1; } - poolUnlock(pool); } void rpmstrPoolUnfreeze(rpmstrPool pool) { if (pool) { - poolLock(pool, 1); + wrlock lock(pool->mutex); if (pool->hash == NULL) { rpmstrPoolRehash(pool); } pool->frozen = 0; - poolUnlock(pool); } } @@ -417,9 +403,13 @@ rpmsid rpmstrPoolIdn(rpmstrPool pool, const char *s, size_t slen, int create) if (pool && s) { unsigned int hash = rstrnhash(s, slen); - poolLock(pool, create); - sid = strn2id(pool, s, slen, hash, create); - poolUnlock(pool); + if (create) { + wrlock lock(pool->mutex); + sid = strn2id(pool, s, slen, hash, create); + } else { + rdlock lock(pool->mutex); + sid = strn2id(pool, s, slen, hash, create); + } } return sid; } @@ -431,9 +421,13 @@ rpmsid rpmstrPoolId(rpmstrPool pool, const char *s, int create) if (pool && s) { size_t slen; unsigned int hash = rstrlenhash(s, &slen); - poolLock(pool, create); - sid = strn2id(pool, s, slen, hash, create); - poolUnlock(pool); + if (create) { + wrlock lock(pool->mutex); + sid = strn2id(pool, s, slen, hash, create); + } else { + rdlock lock(pool->mutex); + sid = strn2id(pool, s, slen, hash, create); + } } return sid; } @@ -442,9 +436,8 @@ const char * rpmstrPoolStr(rpmstrPool pool, rpmsid sid) { const char *s = NULL; if (pool) { - poolLock(pool, 0); + rdlock lock(pool->mutex); s = id2str(pool, sid); - poolUnlock(pool); } return s; } @@ -453,11 +446,10 @@ size_t rpmstrPoolStrlen(rpmstrPool pool, rpmsid sid) { size_t slen = 0; if (pool) { - poolLock(pool, 0); + rdlock lock(pool->mutex); const char *s = id2str(pool, sid); if (s) slen = strlen(s); - poolUnlock(pool); } return slen; } @@ -469,13 +461,11 @@ int rpmstrPoolStreq(rpmstrPool poolA, rpmsid sidA, if (poolA == poolB) eq = (sidA == sidB); else { - poolLock(poolA, 0); - poolLock(poolB, 0); + rdlock lockB(poolB->mutex); + rdlock lockA(poolA->mutex); const char *a = rpmstrPoolStr(poolA, sidA); const char *b = rpmstrPoolStr(poolB, sidB); eq = rstreq(a, b); - poolUnlock(poolA); - poolUnlock(poolB); } return eq; } @@ -484,9 +474,8 @@ rpmsid rpmstrPoolNumStr(rpmstrPool pool) { rpmsid n = 0; if (pool) { - poolLock(pool, 0); + rdlock lock(pool->mutex); n = pool->offs_size; - poolUnlock(pool); } return n; } From 716258512a618fc389d5422cc4d0ffedb3d4eb26 Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Tue, 10 Sep 2024 17:28:25 +0300 Subject: [PATCH 03/10] Take advantage of C++ native mutex facilities for keyring and keys Replace manual pthread_* calls with the STL counterparts. Since there was nothing to wrap these calls to begin with, this makes for a particularly nice cleanup. --- rpmio/rpmkeyring.c | 53 +++++++++++++++------------------------------- 1 file changed, 17 insertions(+), 36 deletions(-) diff --git a/rpmio/rpmkeyring.c b/rpmio/rpmkeyring.c index 0f2d511cc9..fff6123dea 100644 --- a/rpmio/rpmkeyring.c +++ b/rpmio/rpmkeyring.c @@ -1,9 +1,10 @@ #include "system.h" +#include #include +#include #include #include -#include #include #include @@ -21,13 +22,16 @@ struct rpmPubkey_s { std::string keyid; pgpDigParams pgpkey; int nrefs; - pthread_rwlock_t lock; + std::shared_mutex mutex; }; +using wrlock = std::unique_lock; +using rdlock = std::shared_lock; + struct rpmKeyring_s { std::map keys; /* pointers to keys */ int nrefs; - pthread_rwlock_t lock; + std::shared_mutex mutex; }; static std::string key2str(const uint8_t *keyid) @@ -39,7 +43,6 @@ rpmKeyring rpmKeyringNew(void) { rpmKeyring keyring = new rpmKeyring_s {}; keyring->nrefs = 1; - pthread_rwlock_init(&keyring->lock, NULL); return keyring; } @@ -48,15 +51,11 @@ rpmKeyring rpmKeyringFree(rpmKeyring keyring) if (keyring == NULL) return NULL; - pthread_rwlock_wrlock(&keyring->lock); + wrlock lock(keyring->mutex); if (--keyring->nrefs == 0) { for (auto & item : keyring->keys) rpmPubkeyFree(item.second); - pthread_rwlock_unlock(&keyring->lock); - pthread_rwlock_destroy(&keyring->lock); delete keyring; - } else { - pthread_rwlock_unlock(&keyring->lock); } return NULL; } @@ -70,7 +69,7 @@ int rpmKeyringModify(rpmKeyring keyring, rpmPubkey key, rpmKeyringModifyMode mod return -1; /* check if we already have this key, but always wrlock for simplicity */ - pthread_rwlock_wrlock(&keyring->lock); + wrlock lock(keyring->mutex); auto item = keyring->keys.find(key->keyid); if (item != keyring->keys.end() && mode == RPMKEYRING_DELETE) { rpmPubkeyFree(item->second); @@ -84,7 +83,6 @@ int rpmKeyringModify(rpmKeyring keyring, rpmPubkey key, rpmKeyringModifyMode mod keyring->keys.insert({key->keyid, rpmPubkeyLink(key)}); rc = 0; } - pthread_rwlock_unlock(&keyring->lock); return rc; } @@ -98,19 +96,17 @@ rpmPubkey rpmKeyringLookupKey(rpmKeyring keyring, rpmPubkey key) { if (keyring == NULL || key == NULL) return NULL; - pthread_rwlock_rdlock(&keyring->lock); + rdlock lock(keyring->mutex); auto item = keyring->keys.find(key->keyid); rpmPubkey rkey = item == keyring->keys.end() ? NULL : rpmPubkeyLink(item->second); - pthread_rwlock_unlock(&keyring->lock); return rkey; } rpmKeyring rpmKeyringLink(rpmKeyring keyring) { if (keyring) { - pthread_rwlock_wrlock(&keyring->lock); + wrlock lock(keyring->mutex); keyring->nrefs++; - pthread_rwlock_unlock(&keyring->lock); } return keyring; } @@ -152,7 +148,6 @@ rpmPubkey rpmPubkeyNew(const uint8_t *pkt, size_t pktlen) key->nrefs = 1; memcpy(key->pkt.data(), pkt, pktlen); key->keyid = key2str(keyid); - pthread_rwlock_init(&key->lock, NULL); exit: return key; @@ -165,7 +160,6 @@ static rpmPubkey rpmPubkeyNewSubkey(pgpDigParams pgpkey) key->pgpkey = pgpkey; key->keyid = key2str(pgpDigParamsSignID(pgpkey)); key->nrefs = 1; - pthread_rwlock_init(&key->lock, NULL); return key; } @@ -195,14 +189,10 @@ rpmPubkey rpmPubkeyFree(rpmPubkey key) if (key == NULL) return NULL; - pthread_rwlock_wrlock(&key->lock); + wrlock lock(key->mutex); if (--key->nrefs == 0) { pgpDigParamsFree(key->pgpkey); - pthread_rwlock_unlock(&key->lock); - pthread_rwlock_destroy(&key->lock); delete key; - } else { - pthread_rwlock_unlock(&key->lock); } return NULL; } @@ -210,9 +200,8 @@ rpmPubkey rpmPubkeyFree(rpmPubkey key) rpmPubkey rpmPubkeyLink(rpmPubkey key) { if (key) { - pthread_rwlock_wrlock(&key->lock); + wrlock lock(key->mutex); key->nrefs++; - pthread_rwlock_unlock(&key->lock); } return key; } @@ -222,9 +211,8 @@ char * rpmPubkeyBase64(rpmPubkey key) char *enc = NULL; if (key) { - pthread_rwlock_rdlock(&key->lock); + rdlock lock(key->mutex); enc = rpmBase64Encode(key->pkt.data(), key->pkt.size(), -1); - pthread_rwlock_unlock(&key->lock); } return enc; } @@ -236,8 +224,8 @@ rpmRC rpmPubkeyMerge(rpmPubkey oldkey, rpmPubkey newkey, rpmPubkey *mergedkeyp) size_t mergedpktlen = 0; rpmRC rc; - pthread_rwlock_rdlock(&oldkey->lock); - pthread_rwlock_rdlock(&newkey->lock); + rdlock olock(oldkey->mutex); + rdlock nlock(newkey->mutex); rc = pgpPubkeyMerge(oldkey->pkt.data(), oldkey->pkt.size(), newkey->pkt.data(), newkey->pkt.size(), &mergedpkt, &mergedpktlen, 0); if (rc == RPMRC_OK && (mergedpktlen != oldkey->pkt.size() || memcmp(mergedpkt, oldkey->pkt.data(), mergedpktlen) != 0)) { mergedkey = rpmPubkeyNew(mergedpkt, mergedpktlen); @@ -246,8 +234,6 @@ rpmRC rpmPubkeyMerge(rpmPubkey oldkey, rpmPubkey newkey, rpmPubkey *mergedkeyp) } *mergedkeyp = mergedkey; free(mergedpkt); - pthread_rwlock_unlock(&newkey->lock); - pthread_rwlock_unlock(&oldkey->lock); return rc; } @@ -266,6 +252,7 @@ static rpmPubkey findbySig(rpmKeyring keyring, pgpDigParams sig) rpmPubkey key = NULL; if (keyring && sig) { + rdlock lock(keyring->mutex); auto keyid = key2str(pgpDigParamsSignID(sig)); auto item = keyring->keys.find(keyid); if (item != keyring->keys.end()) { @@ -287,9 +274,6 @@ rpmRC rpmKeyringVerifySig(rpmKeyring keyring, pgpDigParams sig, DIGEST_CTX ctx) { rpmRC rc = RPMRC_FAIL; - if (keyring) - pthread_rwlock_rdlock(&keyring->lock); - if (sig && ctx) { pgpDigParams pgpkey = NULL; rpmPubkey key = findbySig(keyring, sig); @@ -306,8 +290,5 @@ rpmRC rpmKeyringVerifySig(rpmKeyring keyring, pgpDigParams sig, DIGEST_CTX ctx) } } - if (keyring) - pthread_rwlock_unlock(&keyring->lock); - return rc; } From dfc0b859a2e3a6ecff58d67b2c0c4e591bb5772f Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Tue, 10 Sep 2024 17:44:39 +0300 Subject: [PATCH 04/10] Take advantage of C++ native mutex facilities for rpmlog See previous commits for rationale. Of particular note here is that we now always take a write lock where we previously dynamically selected between read/write, because the STL primitives don't seem to support that in any easy way. --- rpmio/rpmlog.c | 136 +++++++++++++++++-------------------------------- 1 file changed, 48 insertions(+), 88 deletions(-) diff --git a/rpmio/rpmlog.c b/rpmio/rpmlog.c index e3d2b1211a..331e9a282b 100644 --- a/rpmio/rpmlog.c +++ b/rpmio/rpmlog.c @@ -4,12 +4,13 @@ #include "system.h" +#include +#include #include #include #include #include -#include #include #include #include @@ -18,13 +19,13 @@ typedef struct rpmlogCtx_s * rpmlogCtx; struct rpmlogCtx_s { - pthread_rwlock_t lock; unsigned mask; int nrecsPri[RPMLOG_NPRIS]; std::vector recs; rpmlogCallback cbfunc; rpmlogCallbackData cbdata; FILE *stdlog; + std::shared_mutex mutex; }; struct rpmlogRec_s { @@ -33,40 +34,23 @@ struct rpmlogRec_s { std::string message; /* log message string */ }; +using wrlock = std::unique_lock; +using rdlock = std::shared_lock; + /* Force log context acquisition through a function */ -static rpmlogCtx rpmlogCtxAcquire(int write) +static rpmlogCtx rpmlogCtxAcquire() { - static struct rpmlogCtx_s _globalCtx = { PTHREAD_RWLOCK_INITIALIZER, - RPMLOG_UPTO(RPMLOG_NOTICE), + static struct rpmlogCtx_s _globalCtx = { RPMLOG_UPTO(RPMLOG_NOTICE), {0}, {}, NULL, NULL, NULL }; - rpmlogCtx ctx = &_globalCtx; - int xx; - - /* XXX Silently failing is bad, but we can't very well use log here... */ - if (write) - xx = pthread_rwlock_wrlock(&ctx->lock); - else - xx = pthread_rwlock_rdlock(&ctx->lock); - - return (xx == 0) ? ctx : NULL; -} - -/* Release log context */ -static rpmlogCtx rpmlogCtxRelease(rpmlogCtx ctx) -{ - if (ctx) - pthread_rwlock_unlock(&ctx->lock); - return NULL; + return &_globalCtx; } int rpmlogGetNrecsByMask(unsigned mask) { - rpmlogCtx ctx = rpmlogCtxAcquire(0); + rpmlogCtx ctx = rpmlogCtxAcquire(); int nrecs = 0; - if (ctx == NULL) - return -1; - + rdlock lock(ctx->mutex); if (mask) { for (int i = 0; i < RPMLOG_NPRIS; i++, mask >>= 1) if (mask & 1) @@ -74,7 +58,6 @@ int rpmlogGetNrecsByMask(unsigned mask) } else nrecs = ctx->recs.size(); - rpmlogCtxRelease(ctx); return nrecs; } @@ -86,24 +69,24 @@ int rpmlogGetNrecs(void) int rpmlogCode(void) { int code = -1; - rpmlogCtx ctx = rpmlogCtxAcquire(0); + rpmlogCtx ctx = rpmlogCtxAcquire(); + rdlock lock(ctx->mutex); - if (ctx && ctx->recs.empty() == false) + if (ctx->recs.empty() == false) code = ctx->recs.back().code; - rpmlogCtxRelease(ctx); return code; } const char * rpmlogMessage(void) { const char *msg = _("(no error)"); - rpmlogCtx ctx = rpmlogCtxAcquire(0); + rpmlogCtx ctx = rpmlogCtxAcquire(); + rdlock lock(ctx->mutex); - if (ctx && ctx->recs.empty() == false) + if (ctx->recs.empty() == false) msg = ctx->recs.back().message.c_str(); - rpmlogCtxRelease(ctx); return msg; } @@ -119,10 +102,8 @@ rpmlogLvl rpmlogRecPriority(rpmlogRec rec) void rpmlogPrintByMask(FILE *f, unsigned mask) { - rpmlogCtx ctx = rpmlogCtxAcquire(0); - - if (ctx == NULL) - return; + rpmlogCtx ctx = rpmlogCtxAcquire(); + rdlock lock(ctx->mutex); if (f == NULL) f = stderr; @@ -133,8 +114,6 @@ void rpmlogPrintByMask(FILE *f, unsigned mask) if (rec.message.empty() == false) fprintf(f, " %s", rec.message.c_str()); } - - rpmlogCtxRelease(ctx); } void rpmlogPrint(FILE *f) @@ -144,15 +123,11 @@ void rpmlogPrint(FILE *f) void rpmlogClose (void) { - rpmlogCtx ctx = rpmlogCtxAcquire(1); - - if (ctx == NULL) - return; + rpmlogCtx ctx = rpmlogCtxAcquire(); + wrlock lock(ctx->mutex); ctx->recs.clear(); memset(ctx->nrecsPri, 0, sizeof(ctx->nrecsPri)); - - rpmlogCtxRelease(ctx); } void rpmlogOpen (const char *ident, int option, @@ -162,45 +137,36 @@ void rpmlogOpen (const char *ident, int option, int rpmlogSetMask (int mask) { - rpmlogCtx ctx = rpmlogCtxAcquire(mask ? 1 : 0); + rpmlogCtx ctx = rpmlogCtxAcquire(); - int omask = -1; - if (ctx) { - omask = ctx->mask; - if (mask) - ctx->mask = mask; - } + wrlock lock(ctx->mutex); + int omask = ctx->mask; + if (mask) + ctx->mask = mask; - rpmlogCtxRelease(ctx); return omask; } rpmlogCallback rpmlogSetCallback(rpmlogCallback cb, rpmlogCallbackData data) { - rpmlogCtx ctx = rpmlogCtxAcquire(1); + rpmlogCtx ctx = rpmlogCtxAcquire(); + wrlock lock(ctx->mutex); - rpmlogCallback ocb = NULL; - if (ctx) { - ocb = ctx->cbfunc; - ctx->cbfunc = cb; - ctx->cbdata = data; - } + rpmlogCallback ocb = ctx->cbfunc; + ctx->cbfunc = cb; + ctx->cbdata = data; - rpmlogCtxRelease(ctx); return ocb; } FILE * rpmlogSetFile(FILE * fp) { - rpmlogCtx ctx = rpmlogCtxAcquire(1); + rpmlogCtx ctx = rpmlogCtxAcquire(); + wrlock lock(ctx->mutex); - FILE * ofp = NULL; - if (ctx) { - ofp = ctx->stdlog; - ctx->stdlog = fp; - } + FILE * ofp = ctx->stdlog; + ctx->stdlog = fp; - rpmlogCtxRelease(ctx); return ofp; } @@ -374,17 +340,14 @@ static int rpmlogDefault(FILE *stdlog, rpmlogRec rec) /* FIX: rpmlogMsgPrefix[] may be NULL */ static void dolog(struct rpmlogRec_s *rec, int saverec) { - static pthread_mutex_t serialize = PTHREAD_MUTEX_INITIALIZER; - + static std::mutex serial_mutex; int cbrc = RPMLOG_DEFAULT; int needexit = 0; FILE *clog = NULL; rpmlogCallbackData cbdata = NULL; rpmlogCallback cbfunc = NULL; - rpmlogCtx ctx = rpmlogCtxAcquire(saverec); - - if (ctx == NULL) - return; + rpmlogCtx ctx = rpmlogCtxAcquire(); + wrlock lock(ctx->mutex); /* Save copy of all messages at warning (or below == "more important"). */ if (saverec) { @@ -396,25 +359,22 @@ static void dolog(struct rpmlogRec_s *rec, int saverec) clog = ctx->stdlog; /* Free the context for callback and actual log output */ - ctx = rpmlogCtxRelease(ctx); + lock.unlock(); /* Always serialize callback and output to avoid interleaved messages. */ - if (pthread_mutex_lock(&serialize) == 0) { - if (cbfunc) { - cbrc = cbfunc(rec, cbdata); - needexit += cbrc & RPMLOG_EXIT; - } + std::lock_guard serialize(serial_mutex); + if (cbfunc) { + cbrc = cbfunc(rec, cbdata); + needexit += cbrc & RPMLOG_EXIT; + } - if (cbrc & RPMLOG_DEFAULT) { - cbrc = rpmlogDefault(clog, rec); - needexit += cbrc & RPMLOG_EXIT; - } - pthread_mutex_unlock(&serialize); + if (cbrc & RPMLOG_DEFAULT) { + cbrc = rpmlogDefault(clog, rec); + needexit += cbrc & RPMLOG_EXIT; } - + if (needexit) exit(EXIT_FAILURE); - } void rpmlog (int code, const char *fmt, ...) From 474f435546bb3b7199846abcc8e8f71a625e2c0a Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Tue, 10 Sep 2024 17:49:21 +0300 Subject: [PATCH 05/10] Take advantage of C++ native mutex facilities for keyid tracking See previous commits for rationale. --- lib/package.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/package.c b/lib/package.c index 60943475dc..a80a5f03d3 100644 --- a/lib/package.c +++ b/lib/package.c @@ -4,10 +4,10 @@ #include "system.h" +#include #include #include -#include #include /* XXX RPMSIGTAG, other sig stuff */ #include @@ -117,21 +117,17 @@ rpmTagVal headerMergeLegacySigs(Header h, Header sigh, char **msg) */ static int stashKeyid(unsigned int keyid) { - static pthread_mutex_t keyid_lock = PTHREAD_MUTEX_INITIALIZER; + static std::mutex keyid_mutex; static std::set keyids; int seen = 0; if (keyid == 0) return 0; - /* Just pretend we didn't see the keyid if we fail to lock */ - if (pthread_mutex_lock(&keyid_lock)) - return 0; - + std::lock_guard lock(keyid_mutex); auto ret = keyids.insert(keyid); seen = (ret.second == false); - pthread_mutex_unlock(&keyid_lock); return seen; } From d5063c4a2dc02027f8d4bb0f7c5e63b24684d1cb Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Fri, 13 Sep 2024 09:06:26 +0300 Subject: [PATCH 06/10] Take advantage of C++ native mutex facilities for rpmrc locking See previous commits for rationale. --- lib/rpmrc.c | 63 ++++++++++++++++++++++------------------------------- 1 file changed, 26 insertions(+), 37 deletions(-) diff --git a/lib/rpmrc.c b/lib/rpmrc.c index 5d778b8b78..76f2b09708 100644 --- a/lib/rpmrc.c +++ b/lib/rpmrc.c @@ -1,8 +1,10 @@ #include "system.h" +#include +#include + #include #include -#include #if defined(__linux__) #include @@ -40,6 +42,9 @@ #include "debug.h" +using wrlock = std::unique_lock; +using rdlock = std::shared_lock; + static const char * defrcfiles = NULL; const char * macrofiles = NULL; @@ -145,7 +150,7 @@ struct rpmrcCtx_s { struct tableType_s tables[RPM_MACHTABLE_COUNT]; int machDefaults; int pathDefaults; - pthread_rwlock_t lock; + std::shared_mutex mutex; }; /* prototypes */ @@ -158,8 +163,8 @@ static void rebuildCompatTables(rpmrcCtx ctx, int type, const char * name); static void rpmRebuildTargetVars(rpmrcCtx ctx, const char **target, const char ** canontarget); -/* Force context (lock) acquisition through a function */ -static rpmrcCtx rpmrcCtxAcquire(int write) +/* Force context acquisition through a function */ +static rpmrcCtx rpmrcCtxAcquire() { static struct rpmrcCtx_s _globalCtx = { .currTables = { RPM_MACHTABLE_INSTOS, RPM_MACHTABLE_INSTARCH }, @@ -169,26 +174,12 @@ static rpmrcCtx rpmrcCtxAcquire(int write) { "buildarch", 0, 1 }, { "buildos", 0, 1 } }, - .lock = PTHREAD_RWLOCK_INITIALIZER, }; rpmrcCtx ctx = &_globalCtx; - /* XXX: errors should be handled */ - if (write) - pthread_rwlock_wrlock(&ctx->lock); - else - pthread_rwlock_rdlock(&ctx->lock); - return ctx; } -/* Release context (lock) */ -static rpmrcCtx rpmrcCtxRelease(rpmrcCtx ctx) -{ - pthread_rwlock_unlock(&ctx->lock); - return NULL; -} - static int optionCompare(const void * a, const void * b) { return rstrcasecmp(((const struct rpmOption *) a)->name, @@ -1791,7 +1782,8 @@ static rpmRC rpmReadRC(rpmrcCtx ctx, const char * rcfiles) int rpmReadConfigFiles(const char * file, const char * target) { int rc = -1; /* assume failure */ - rpmrcCtx ctx = rpmrcCtxAcquire(1); + rpmrcCtx ctx = rpmrcCtxAcquire(); + wrlock lock(ctx->mutex); if (rpmInitCrypto()) goto exit; @@ -1826,13 +1818,13 @@ int rpmReadConfigFiles(const char * file, const char * target) rc = 0; exit: - rpmrcCtxRelease(ctx); return rc; } void rpmFreeRpmrc(void) { - rpmrcCtx ctx = rpmrcCtxAcquire(1); + rpmrcCtx ctx = rpmrcCtxAcquire(); + wrlock lock(ctx->mutex); int i, j, k; ctx->platpat = argvFree(ctx->platpat); @@ -1902,14 +1894,14 @@ void rpmFreeRpmrc(void) rpmluaFree(lua); rpmugFree(); - rpmrcCtxRelease(ctx); return; } int rpmShowRC(FILE * fp) { - /* Write-context necessary as this calls rpmSetTables(), ugh */ - rpmrcCtx ctx = rpmrcCtxAcquire(1); + rpmrcCtx ctx = rpmrcCtxAcquire(); + /* Write-lock necessary as this calls rpmSetTables(), ugh */ + wrlock lock(ctx->mutex); const struct rpmOption *opt; rpmds ds = NULL; int i; @@ -1977,9 +1969,6 @@ int rpmShowRC(FILE * fp) rpmDumpMacroTable(NULL, fp); - /* XXX: Move this earlier eventually... */ - rpmrcCtxRelease(ctx); - return 0; } @@ -1987,36 +1976,37 @@ int rpmMachineScore(int type, const char * name) { int score = 0; if (name) { - rpmrcCtx ctx = rpmrcCtxAcquire(0); + rpmrcCtx ctx = rpmrcCtxAcquire(); + rdlock lock(ctx->mutex); machEquivInfo info = machEquivSearch(&ctx->tables[type].equiv, name); if (info) score = info->score; - rpmrcCtxRelease(ctx); } return score; } int rpmIsKnownArch(const char *name) { - rpmrcCtx ctx = rpmrcCtxAcquire(0); + rpmrcCtx ctx = rpmrcCtxAcquire(); + rdlock lock(ctx->mutex); canonEntry canon = lookupInCanonTable(name, ctx->tables[RPM_MACHTABLE_INSTARCH].canons, ctx->tables[RPM_MACHTABLE_INSTARCH].canonsLength); int known = (canon != NULL || rstreq(name, "noarch")); - rpmrcCtxRelease(ctx); return known; } void rpmGetArchInfo(const char ** name, int * num) { - rpmrcCtx ctx = rpmrcCtxAcquire(0); + rpmrcCtx ctx = rpmrcCtxAcquire(); + rdlock lock(ctx->mutex); getMachineInfo(ctx, ARCH, name, num); - rpmrcCtxRelease(ctx); } int rpmGetArchColor(const char *arch) { - rpmrcCtx ctx = rpmrcCtxAcquire(0); + rpmrcCtx ctx = rpmrcCtxAcquire(); + rdlock lock(ctx->mutex); const char *color; char *e; int color_i = -1; /* assume failure */ @@ -2031,15 +2021,14 @@ int rpmGetArchColor(const char *arch) color_i = -1; } } - rpmrcCtxRelease(ctx); return color_i; } void rpmGetOsInfo(const char ** name, int * num) { - rpmrcCtx ctx = rpmrcCtxAcquire(0); + rpmrcCtx ctx = rpmrcCtxAcquire(); + rdlock lock(ctx->mutex); getMachineInfo(ctx, OS, name, num); - rpmrcCtxRelease(ctx); } From 1457e17cdf70c8cd70ec5d42f965673eb6a20d21 Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Tue, 24 Sep 2024 09:44:15 +0300 Subject: [PATCH 07/10] Eliminate an unnecessary round of locking from every rpmlog() call On entry to rpmlog() we compare the log mask to see if it's something we need to act on at all. Since we've been using rpmlogSetMask() for this, each and every RPMLOG_DEBUG etc call that is not normally logged stops on its way to take at least one read mutex to do nothing at all. Which is nuts. rpmlogSetMask() technically does need the mutex lock because it both reads and writes, and something could come in between. But for the rpmlog() entry we only need to read, and reading an int is atomic. Add an internal helper that lets us get the silly mask without locking. --- rpmio/rpmlog.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/rpmio/rpmlog.c b/rpmio/rpmlog.c index 331e9a282b..6253e45d51 100644 --- a/rpmio/rpmlog.c +++ b/rpmio/rpmlog.c @@ -377,6 +377,13 @@ static void dolog(struct rpmlogRec_s *rec, int saverec) exit(EXIT_FAILURE); } +static int rpmlogGetMask(void) +{ + /* No locking needed, reading an int is atomic */ + rpmlogCtx ctx = rpmlogCtxAcquire(); + return ctx->mask; +} + void rpmlog (int code, const char *fmt, ...) { int saved_errno = errno; @@ -386,7 +393,7 @@ void rpmlog (int code, const char *fmt, ...) va_list ap; char *msg = NULL; - if ((mask & rpmlogSetMask(0)) == 0) + if ((mask & rpmlogGetMask()) == 0) goto exit; va_start(ap, fmt); From d16ad8ddcfc8064dbbcb252a322e1b2ca6c30430 Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Fri, 13 Sep 2024 09:08:04 +0300 Subject: [PATCH 08/10] Drop a leftover pthread.h include in rpmvs The initial implementation in 4e158f5d4ce6c1464d6c36b735ae5d8c28b87ed6 used pthread once-only initialization for verifylevel but that got dropped soon after, only the include remains. --- lib/rpmvs.c | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/rpmvs.c b/lib/rpmvs.c index 783dbba874..8bea313516 100644 --- a/lib/rpmvs.c +++ b/lib/rpmvs.c @@ -1,6 +1,5 @@ #include "system.h" -#include #include #include #include From 5487968fcfda1fca2f6591a2eca9c34ded941773 Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Fri, 13 Sep 2024 09:23:36 +0300 Subject: [PATCH 09/10] Eliminate pthread_once from global config dir initialization C++ guarantees static initializers run in a thread-safe manner, move the confdir initialization to a constructor of a tiny object and voila we don't need the pthread once dance. --- rpmio/rpmfileutil.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/rpmio/rpmfileutil.c b/rpmio/rpmfileutil.c index 32b68c5173..8e47e972df 100644 --- a/rpmio/rpmfileutil.c +++ b/rpmio/rpmfileutil.c @@ -1,6 +1,7 @@ #include "system.h" #include +#include #include #include @@ -9,7 +10,6 @@ #include #include #include -#include #include #include @@ -23,9 +23,6 @@ namespace fs = std::filesystem; -static const char *rpm_config_dir = NULL; -static pthread_once_t configDirSet = PTHREAD_ONCE_INIT; - int rpmDoDigest(int algo, const char * fn,int asAscii, unsigned char * digest) { unsigned char * dig = NULL; @@ -476,14 +473,17 @@ int rpmMkdirs(const char *root, const char *pathstr) return rc; } -static void setConfigDir(void) -{ - char *rpmenv = getenv("RPM_CONFIGDIR"); - rpm_config_dir = rpmenv ? xstrdup(rpmenv) : RPM_CONFIGDIR; -} +/* One-shot initialization of our global config directory */ +struct rpmConfDir { + std::string path; + rpmConfDir() { + char *rpmenv = getenv("RPM_CONFIGDIR"); + path = rpmenv ? rpmenv : RPM_CONFIGDIR; + }; +}; const char *rpmConfigDir(void) { - pthread_once(&configDirSet, setConfigDir); - return rpm_config_dir; + static rpmConfDir confDir {}; + return confDir.path.c_str(); } From d5236d7faee0d6d336973b787e3943a4831bec5d Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Fri, 13 Sep 2024 09:45:42 +0300 Subject: [PATCH 10/10] Eliminate pthread_once in tag table preparation Move the tag table initialization to static object constructor so it runs before there are threads to worry about. This will slightly increase our startup time for the arbitrary 'rpm --eval "%foo"' type use but unlikely to matter in practise. To do this, minimally objectify our tag entry table. The resulting code division doesn't make too much sense but that can be cleaned up later. --- lib/tagname.c | 102 ++++++++++++++++++++++++++------------------------ 1 file changed, 53 insertions(+), 49 deletions(-) diff --git a/lib/tagname.c b/lib/tagname.c index 0d49599c4d..f42a57de63 100644 --- a/lib/tagname.c +++ b/lib/tagname.c @@ -3,9 +3,6 @@ */ #include "system.h" - -#include - #include #include #include "debug.h" @@ -28,9 +25,6 @@ struct headerTagTableEntry_s { #define TABLESIZE (sizeof(rpmTagTable) / sizeof(rpmTagTable[0]) - 1) static const int rpmTagTableSize = TABLESIZE; -static headerTagTableEntry tagsByName[TABLESIZE]; /*!< tags sorted by name. */ -static headerTagTableEntry tagsByValue[TABLESIZE]; /*!< tags sorted by value. */ - /** * Compare tag table entries by name. * @param *avp tag table entry a @@ -61,21 +55,32 @@ static int tagCmpValue(const void * avp, const void * bvp) return ret; } -static pthread_once_t tagsLoaded = PTHREAD_ONCE_INIT; +class tagTable { + public: + tagTable(); + headerTagTableEntry getEntry(const char *tag); + headerTagTableEntry getEntry(rpmTagVal tag); + int getNames(rpmtd tagnames, int fullname); + + private: + headerTagTableEntry byName[TABLESIZE]; /*!< tags sorted by name */ + headerTagTableEntry byValue[TABLESIZE]; /*!< tags sorted by value */ + +}; -/* Initialize tag by-value and by-name lookup tables */ -static void loadTags(void) +tagTable::tagTable() { + /* Initialize tag by-value and by-name lookup tables */ for (int i = 0; i < rpmTagTableSize; i++) { - tagsByValue[i] = &rpmTagTable[i]; - tagsByName[i] = &rpmTagTable[i]; + byValue[i] = &rpmTagTable[i]; + byName[i] = &rpmTagTable[i]; } - qsort(tagsByValue, rpmTagTableSize, sizeof(*tagsByValue), tagCmpValue); - qsort(tagsByName, rpmTagTableSize, sizeof(*tagsByName), tagCmpName); + qsort(byValue, rpmTagTableSize, sizeof(*byValue), tagCmpValue); + qsort(byName, rpmTagTableSize, sizeof(*byName), tagCmpName); } -static headerTagTableEntry entryByTag(rpmTagVal tag) +headerTagTableEntry tagTable::getEntry(rpmTagVal tag) { headerTagTableEntry entry = NULL; int i, comparison; @@ -84,7 +89,7 @@ static headerTagTableEntry entryByTag(rpmTagVal tag) while (l < u) { i = (l + u) / 2; - comparison = (tag - tagsByValue[i]->val); + comparison = (tag - byValue[i]->val); if (comparison < 0) { u = i; @@ -92,17 +97,17 @@ static headerTagTableEntry entryByTag(rpmTagVal tag) l = i + 1; } else { /* Make sure that the bsearch retrieve is stable. */ - while (i > 0 && tag == tagsByValue[i-1]->val) { + while (i > 0 && tag == byValue[i-1]->val) { i--; } - entry = tagsByValue[i]; + entry = byValue[i]; break; } } return entry; } -static headerTagTableEntry entryByName(const char *tag) +headerTagTableEntry tagTable::getEntry(const char *tag) { headerTagTableEntry entry = NULL; int i, comparison; @@ -111,27 +116,49 @@ static headerTagTableEntry entryByName(const char *tag) while (l < u) { i = (l + u) / 2; - comparison = rstrcasecmp(tag, tagsByName[i]->shortname); + comparison = rstrcasecmp(tag, byName[i]->shortname); if (comparison < 0) { u = i; } else if (comparison > 0) { l = i + 1; } else { - entry = tagsByName[i]; + entry = byName[i]; break; } } return entry; } +int tagTable::getNames(rpmtd tagnames, int fullname) +{ + const char **names; + const char *name; + + if (tagnames == NULL) + return 0; + + rpmtdReset(tagnames); + tagnames->count = rpmTagTableSize; + tagnames->data = names = (const char **)xmalloc(tagnames->count * sizeof(*names)); + tagnames->type = RPM_STRING_ARRAY_TYPE; + tagnames->flags = RPMTD_ALLOCED | RPMTD_IMMUTABLE; + + for (int i = 0; i < tagnames->count; i++) { + name = fullname ? byName[i]->name : + byName[i]->shortname; + names[i] = name; + } + return tagnames->count; +} + +static tagTable tags; + const char * rpmTagGetName(rpmTagVal tag) { const char *name = "(unknown)"; const struct headerTagTableEntry_s *t; - pthread_once(&tagsLoaded, loadTags); - switch (tag) { case RPMDBI_PACKAGES: name = "Packages"; @@ -145,7 +172,7 @@ const char * rpmTagGetName(rpmTagVal tag) break; default: - t = entryByTag(tag); + t = tags.getEntry(tag); if (t && t->shortname) name = t->shortname; break; @@ -158,9 +185,7 @@ rpmTagType rpmTagGetType(rpmTagVal tag) const struct headerTagTableEntry_s *t; rpmTagType tagtype = RPM_NULL_TYPE; - pthread_once(&tagsLoaded, loadTags); - - t = entryByTag(tag); + t = tags.getEntry(tag); if (t) { /* XXX this is dumb */ tagtype = (rpmTagType)(t->type | t->retype); @@ -173,12 +198,10 @@ rpmTagVal rpmTagGetValue(const char * tagstr) const struct headerTagTableEntry_s *t; rpmTagVal tagval = RPMTAG_NOT_FOUND; - pthread_once(&tagsLoaded, loadTags); - if (!rstrcasecmp(tagstr, "Packages")) return RPMDBI_PACKAGES; - t = entryByName(tagstr); + t = tags.getEntry(tagstr); if (t) tagval = t->val; @@ -229,24 +252,5 @@ rpmTagClass rpmTagGetClass(rpmTagVal tag) int rpmTagGetNames(rpmtd tagnames, int fullname) { - const char **names; - const char *name; - - pthread_once(&tagsLoaded, loadTags); - - if (tagnames == NULL) - return 0; - - rpmtdReset(tagnames); - tagnames->count = rpmTagTableSize; - tagnames->data = names = (const char **)xmalloc(tagnames->count * sizeof(*names)); - tagnames->type = RPM_STRING_ARRAY_TYPE; - tagnames->flags = RPMTD_ALLOCED | RPMTD_IMMUTABLE; - - for (int i = 0; i < tagnames->count; i++) { - name = fullname ? tagsByName[i]->name : - tagsByName[i]->shortname; - names[i] = name; - } - return tagnames->count; + return tags.getNames(tagnames, fullname); }