Skip to content

Commit

Permalink
Use vectors for returning multiple values from rpmalAll*()
Browse files Browse the repository at this point in the history 
These are internal interfaces so we're free to change them, avoid
manual allocations and bookkeeping.

Might be worth noting the use of pointers rather than references
in the for loops: these *are* raw pointers so there's no penalty to
doing so, and we're explicitly comparing them to other pointers so...
  • Loading branch information
@pmatilai
pmatilai committed May 7, 2024
1 parent 18f7e53 commit 6b81489
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 74 deletions.
38 changes: 14 additions & 24 deletions lib/depends.c
View file
Expand Up @@ -269,12 +269,9 @@ static int addObsoleteErasures(rpmts ts, rpm_color_t tscolor, rpmte p)
static rpmte checkObsoleted(rpmal addedPackages, rpmds thisds)
{
rpmte p = NULL;
rpmte *matches = NULL;

matches = rpmalAllObsoletes(addedPackages, thisds);
if (matches) {
auto matches = rpmalAllObsoletes(addedPackages, thisds);
if (!matches.empty()) {
p = matches[0];
free(matches);
}
return p;
}
Expand All @@ -289,27 +286,24 @@ static rpmte checkAdded(rpmal addedPackages, rpm_color_t tscolor,
rpmte te, rpmds ds)
{
rpmte p = NULL;
rpmte *matches = NULL;
auto const matches = rpmalAllSatisfiesDepend(addedPackages, ds);

matches = rpmalAllSatisfiesDepend(addedPackages, ds);
if (matches) {
if (!matches.empty()) {
const char * arch = rpmteA(te);
const char * os = rpmteO(te);

for (rpmte *m = matches; m && *m; m++) {
for (rpmte const m : matches) {
if (tscolor) {
const char * parch = rpmteA(*m);
const char * pos = rpmteO(*m);
const char * parch = rpmteA(m);
const char * pos = rpmteO(m);

if (arch == NULL || parch == NULL || os == NULL || pos == NULL)
continue;
if (!rstreq(arch, parch) || !rstreq(os, pos))
continue;
}
p = *m;
p = m;
break;
}
free(matches);
}
}
return p;
}
Expand Down Expand Up @@ -667,12 +661,10 @@ static dbiIndexSet unsatisfiedDependSet(rpmts ts, rpmds dep)

/* Pretrans dependencies can't be satisfied by added packages. */
if (!(dsflags & (RPMSENSE_PRETRANS|RPMSENSE_PREUNTRANS))) {
rpmte *matches = rpmalAllSatisfiesDepend(tsmem->addedPackages, dep);
if (matches) {
for (rpmte *p = matches; *p; p++)
dbiIndexSetAppendOne(set1, rpmalLookupTE(tsmem->addedPackages, *p), 1, 0);
auto matches = rpmalAllSatisfiesDepend(tsmem->addedPackages, dep);
for (rpmte const p : matches) {
dbiIndexSetAppendOne(set1, rpmalLookupTE(tsmem->addedPackages, p), 1, 0);
}
_free(matches);
}

exit:
Expand Down Expand Up @@ -781,10 +773,8 @@ static int unsatisfiedDepend(rpmts ts, depCache dcache, rpmds dep)

/* Pretrans dependencies can't be satisfied by added packages. */
if (!(dsflags & (RPMSENSE_PRETRANS|RPMSENSE_PREUNTRANS))) {
rpmte *matches = rpmalAllSatisfiesDepend(tsmem->addedPackages, dep);
int match = matches && *matches;
_free(matches);
if (match)
auto const matches = rpmalAllSatisfiesDepend(tsmem->addedPackages, dep);
if (!matches.empty())
goto exit;
}

Expand Down
72 changes: 25 additions & 47 deletions lib/rpmal.c
View file
Expand Up @@ -329,9 +329,9 @@ static void rpmalMakeObsoletesIndex(rpmal al)
}
}

rpmte * rpmalAllObsoletes(rpmal al, rpmds ds)
std::vector<rpmte> rpmalAllObsoletes(rpmal al, rpmds ds)
{
rpmte * ret = NULL;
std::vector<rpmte> ret;
rpmsid nameId;
availableIndexEntry result;
int resultCnt;
Expand All @@ -346,41 +346,32 @@ rpmte * rpmalAllObsoletes(rpmal al, rpmds ds)

if (resultCnt > 0) {
availablePackage alp;
int rc, found = 0;

ret = (rpmte *)xmalloc((resultCnt+1) * sizeof(*ret));

for (int i = 0; i < resultCnt; i++) {
alp = al->list + result[i].pkgNum;
if (alp->p == NULL) // deleted
continue;

rc = rpmdsCompareIndex(alp->obsoletes, result[i].entryIx,
int rc = rpmdsCompareIndex(alp->obsoletes, result[i].entryIx,
ds, rpmdsIx(ds));

if (rc) {
rpmdsNotify(ds, "(added obsolete)", 0);
ret[found] = alp->p;
found++;
ret.push_back(alp->p);
}
}

if (found)
ret[found] = NULL;
else
ret = _free(ret);
}

return ret;
}

static rpmte * rpmalAllFileSatisfiesDepend(const rpmal al, const char *fileName, const rpmds filterds)
static std::vector<rpmte> rpmalAllFileSatisfiesDepend(const rpmal al, const char *fileName, const rpmds filterds)
{
const char *slash;
rpmte * ret = NULL;
std::vector<rpmte> ret;

if (al == NULL || fileName == NULL || *fileName != '/')
return NULL;
return ret;

/* Split path into dirname and basename components for lookup */
if ((slash = strrchr(fileName, '/')) != NULL) {
Expand All @@ -394,17 +385,15 @@ static rpmte * rpmalAllFileSatisfiesDepend(const rpmal al, const char *fileName,

baseName = rpmstrPoolId(al->pool, fileName + bnStart, 0);
if (!baseName)
return NULL; /* no match possible */
return ret; /* no match possible */

rpmalFileHashGetEntry(al->fileHash, baseName, &result, &resultCnt, NULL);

if (resultCnt > 0) {
int i, found;
ret = (rpmte *)xmalloc((resultCnt+1) * sizeof(*ret));
fingerPrint * fp = NULL;
rpmsid dirName = rpmstrPoolIdn(al->pool, fileName, bnStart, 1);

for (found = i = 0; i < resultCnt; i++) {
for (int i = 0; i < resultCnt; i++) {
availablePackage alp = al->list + result[i].pkgNum;
if (alp->p == NULL) /* deleted */
continue;
Expand All @@ -420,21 +409,18 @@ static rpmte * rpmalAllFileSatisfiesDepend(const rpmal al, const char *fileName,
if (!fpLookupEqualsId(al->fpc, fp, result[i].dirName, baseName))
continue;
}
ret[found] = alp->p;
found++;
ret.push_back(alp->p);
}
_free(fp);
ret[found] = NULL;
}
}

return ret;
}

rpmte * rpmalAllSatisfiesDepend(const rpmal al, const rpmds ds)
std::vector<rpmte> rpmalAllSatisfiesDepend(const rpmal al, const rpmds ds)
{
rpmte * ret = NULL;
int i, ix, found;
std::vector<rpmte> ret;
rpmsid nameId;
const char *name;
availableIndexEntry result;
Expand All @@ -457,12 +443,11 @@ rpmte * rpmalAllSatisfiesDepend(const rpmal al, const rpmds ds)
if (!obsolete && *name == '/') {
/* First, look for files "contained" in package ... */
ret = rpmalAllFileSatisfiesDepend(al, name, filterds);
if (ret != NULL && *ret != NULL) {
if (!ret.empty()) {
rpmdsNotify(ds, "(added files)", 0);
return ret;
}
/* ... then, look for files "provided" by package. */
ret = _free(ret);
}

if (al->providesHash == NULL)
Expand All @@ -471,18 +456,14 @@ rpmte * rpmalAllSatisfiesDepend(const rpmal al, const rpmds ds)
rpmalDepHashGetEntry(al->providesHash, nameId, &result,
&resultCnt, NULL);

if (resultCnt==0) return NULL;

ret = (rpmte *)xmalloc((resultCnt+1) * sizeof(*ret));

for (found=i=0; i<resultCnt; i++) {
for (int i=0; i<resultCnt; i++) {
alp = al->list + result[i].pkgNum;
if (alp->p == NULL) /* deleted */
continue;
/* ignore self-conflicts/obsoletes */
if (filterds && rpmteDS(alp->p, rpmdsTagN(filterds)) == filterds)
continue;
ix = result[i].entryIx;
int ix = result[i].entryIx;

if (obsolete) {
/* Obsoletes are on package NEVR only */
Expand All @@ -496,14 +477,11 @@ rpmte * rpmalAllSatisfiesDepend(const rpmal al, const rpmds ds)
}

if (rc)
ret[found++] = alp->p;
ret.push_back(alp->p);
}

if (found) {
if (!ret.empty()) {
rpmdsNotify(ds, "(added provide)", 0);
ret[found] = NULL;
} else {
ret = _free(ret);
}

return ret;
Expand All @@ -512,21 +490,21 @@ rpmte * rpmalAllSatisfiesDepend(const rpmal al, const rpmds ds)
rpmte
rpmalSatisfiesDepend(const rpmal al, const rpmte te, const rpmds ds)
{
rpmte *providers = rpmalAllSatisfiesDepend(al, ds);
auto const providers = rpmalAllSatisfiesDepend(al, ds);
rpmte best = NULL;
int bestscore = 0;

if (providers) {
if (!providers.empty()) {
rpm_color_t dscolor = rpmdsColor(ds);
for (rpmte *p = providers; *p; p++) {
for (rpmte const p : providers) {
int score = 0;

/*
* For colored dependencies, prefer a matching colored provider.
* Otherwise prefer provider of ts preferred color.
*/
if (al->tscolor) {
rpm_color_t tecolor = rpmteColor(*p);
rpm_color_t tecolor = rpmteColor(p);
if (dscolor) {
if (dscolor == tecolor) score += 2;
} else if (al->prefcolor) {
Expand All @@ -535,17 +513,17 @@ rpmalSatisfiesDepend(const rpmal al, const rpmte te, const rpmds ds)
}

/* Being self-provided is a bonus */
if (*p == te)
if (p == te)
score += 1;

if (score > bestscore) {
bestscore = score;
best = *p;
best = p;
}
}
/* if not decided by now, just pick first match */
if (!best) best = providers[0];
free(providers);
if (!best)
best = providers[0];
}
return best;
}
Expand Down
7 changes: 4 additions & 3 deletions lib/rpmal.h
View file
Expand Up @@ -6,6 +6,7 @@
* Structures used for managing added/available package lists.
*/

#include <vector>
#include <rpm/rpmtypes.h>
#include <rpm/rpmts.h>

Expand Down Expand Up @@ -51,16 +52,16 @@ void rpmalAdd(rpmal al, rpmte p);
* @return obsoleting packages for ds, NULL if none
*/
RPM_GNUC_INTERNAL
rpmte * rpmalAllObsoletes(const rpmal al, const rpmds ds);
std::vector<rpmte> rpmalAllObsoletes(const rpmal al, const rpmds ds);

/**
* Lookup all providers for a dependency in the available list
* @param al available list
* @param ds dependency set
* @return best provider for the dependency, NULL if none
* @return best provider for the dependency (if any)
*/
RPM_GNUC_INTERNAL
rpmte * rpmalAllSatisfiesDepend(const rpmal al, const rpmds ds);
std::vector<rpmte> rpmalAllSatisfiesDepend(const rpmal al, const rpmds ds);

/**
* Lookup best provider for a dependency in the available list
Expand Down

0 comments on commit 6b81489

Please sign in to comment.