Validate + sort relocations in rpmte at create time

- instead of storing "raw" relocations in rpmte to be passed down to
  rpmRelocateFileList() for copying over and sorting etc, do the work
  just once at rpmteNew() time
- much of the real work to relocate header contents still needs to be done
  twice as the header gets thrown and needs to be reconstructed in
  rpmtsProcess()

diff --git a/lib/rpmfi.c b/lib/rpmfi.c
index 2bdfc77..f30e4e7 100644 (file)
--- a/lib/rpmfi.c
+++ b/lib/rpmfi.c
@@ -747,16 +747,35 @@ static char **duparray(char ** src, int size)
     return dest;
 }
 
-static void addPrefixes(Header h, rpmtd validRelocs,
-                       rpmRelocation *relocations, int numRelocations)
+static int addPrefixes(Header h, rpmRelocation *relocations, int numRelocations)
 {
+    struct rpmtd_s validRelocs;
     const char *validprefix;
     const char ** actualRelocations;
-    int numActual;
+    int numActual = 0;
 
-    actualRelocations = xmalloc(rpmtdCount(validRelocs) * sizeof(*actualRelocations));
-    rpmtdInit(validRelocs);
-    while ((validprefix = rpmtdNextString(validRelocs))) {
+    headerGet(h, RPMTAG_PREFIXES, &validRelocs, HEADERGET_MINMEM);
+    /*
+     * If no relocations are specified (usually the case), then return the
+     * original header. If there are prefixes, however, then INSTPREFIXES
+     * should be added for RPM_INSTALL_PREFIX environ variables in scriptlets, 
+     * but, since relocateFileList() can be called more than once for 
+     * the same header, don't bother if already present.
+     */
+    if (relocations == NULL || numRelocations == 0) {
+       if (rpmtdCount(&validRelocs) > 0) {
+           if (!headerIsEntry(h, RPMTAG_INSTPREFIXES)) {
+               rpmtdSetTag(&validRelocs, RPMTAG_INSTPREFIXES);
+               headerPut(h, &validRelocs, HEADERPUT_DEFAULT);
+           }
+           rpmtdFreeData(&validRelocs);
+       }
+       return 0;
+    }
+
+    actualRelocations = xmalloc(rpmtdCount(&validRelocs) * sizeof(*actualRelocations));
+    rpmtdInit(&validRelocs);
+    while ((validprefix = rpmtdNextString(&validRelocs))) {
        int j;
        for (j = 0; j < numRelocations; j++) {
            if (relocations[j].oldPath == NULL || /* XXX can't happen */
@@ -774,32 +793,13 @@ static void addPrefixes(Header h, rpmtd validRelocs,
            numActual++;
        }
     }
+    rpmtdFreeData(&validRelocs);
 
     if (numActual) {
        headerPutStringArray(h, RPMTAG_INSTPREFIXES, actualRelocations, numActual);
     }
     actualRelocations = _free(actualRelocations);
-}
-
-/* stupid bubble sort, but it's probably faster here */
-static void sortRelocs(rpmRelocation *relocations, int numRelocations)
-{
-    for (int i = 0; i < numRelocations; i++) {
-       int madeSwap = 0;
-       for (int j = 1; j < numRelocations; j++) {
-           rpmRelocation tmpReloc;
-           if (relocations[j - 1].oldPath == NULL || /* XXX can't happen */
-               relocations[j    ].oldPath == NULL || /* XXX can't happen */
-               strcmp(relocations[j - 1].oldPath, relocations[j].oldPath) <= 0)
-               continue;
-           /* LCL: ??? */
-           tmpReloc = relocations[j - 1];
-           relocations[j - 1] = relocations[j];
-           relocations[j] = tmpReloc;
-           madeSwap = 1;
-       }
-       if (!madeSwap) break;
-    }
+    return numActual;
 }
 
 static void saveRelocs(Header h, rpmtd bnames, rpmtd dnames, rpmtd dindexes)
@@ -828,111 +828,28 @@ static void saveRelocs(Header h, rpmtd bnames, rpmtd dnames, rpmtd dindexes)
 /**
  * Relocate files in header.
  * @todo multilib file dispositions need to be checked.
- * @param ts           transaction set
  * @param p            transaction element
  * @param relocs       relocations
  * @param h            package header to relocate
  */
-void rpmRelocateFileList(rpmts ts, rpmte p, 
-                        rpmRelocation *relocs, int numRelocations, Header h)
+void rpmRelocateFileList(rpmte p, 
+                        rpmRelocation *relocations, int numRelocations, Header h)
 {
     static int _printed = 0;
-    int allowBadRelocate = (rpmtsFilterFlags(ts) & RPMPROB_FILTER_FORCERELOCATE);
-    rpmRelocation * relocations = NULL;
     char ** baseNames;
     char ** dirNames;
     uint32_t * dirIndexes;
-    rpm_count_t fileCount, dirCount, numValid = 0;
+    rpm_count_t fileCount, dirCount;
     int nrelocated = 0;
     int fileAlloced = 0;
     char * fn = NULL;
     int haveRelocatedBase = 0;
-    int reldel = 0;
-    int len;
+    size_t maxlen = 0;
     int i, j;
-    struct rpmtd_s validRelocs;
     struct rpmtd_s bnames, dnames, dindexes, fmodes;
 
-    
-    if (headerGet(h, RPMTAG_PREFIXES, &validRelocs, HEADERGET_MINMEM)) 
-       numValid = rpmtdCount(&validRelocs);
-
-    /*
-     * If no relocations are specified (usually the case), then return the
-     * original header. If there are prefixes, however, then INSTPREFIXES
-     * should be added, but, since relocateFileList() can be called more
-     * than once for the same header, don't bother if already present.
-     */
-    if (relocs == NULL || numRelocations == 0) {
-       if (numValid) {
-           if (!headerIsEntry(h, RPMTAG_INSTPREFIXES)) {
-               rpmtdSetTag(&validRelocs, RPMTAG_INSTPREFIXES);
-               headerPut(h, &validRelocs, HEADERPUT_DEFAULT);
-           }
-           rpmtdFreeData(&validRelocs);
-       }
-       /* XXX FIXME multilib file actions need to be checked. */
+    if (addPrefixes(h, relocations, numRelocations) == 0)
        return;
-    }
-
-    relocations = xmalloc(sizeof(*relocations) * numRelocations);
-
-    /* Build sorted relocation list from raw relocations. */
-    for (i = 0; i < numRelocations; i++) {
-       char * t;
-
-       /*
-        * Default relocations (oldPath == NULL) are handled in the UI,
-        * not rpmlib.
-        */
-       if (relocs[i].oldPath == NULL) continue; /* XXX can't happen */
-
-       /* FIXME: Trailing /'s will confuse us greatly. Internal ones will 
-          too, but those are more trouble to fix up. :-( */
-       t = xstrdup(relocs[i].oldPath);
-       relocations[i].oldPath = (t[0] == '/' && t[1] == '\0')
-           ? t
-           : stripTrailingChar(t, '/');
-
-       /* An old path w/o a new path is valid, and indicates exclusion */
-       if (relocs[i].newPath) {
-           int del;
-           int valid = 0;
-           const char *validprefix;
-
-           t = xstrdup(relocs[i].newPath);
-           relocations[i].newPath = (t[0] == '/' && t[1] == '\0')
-               ? t
-               : stripTrailingChar(t, '/');
-
-               /* FIX:  relocations[i].oldPath == NULL */
-           /* Verify that the relocation's old path is in the header. */
-           rpmtdInit(&validRelocs);
-           while ((validprefix = rpmtdNextString(&validRelocs))) {
-               if (strcmp(validprefix, relocations[i].oldPath) == 0) {
-                   valid = 1;
-                   break;
-               }
-           }
-
-           if (!valid && !allowBadRelocate) {
-               rpmps ps = rpmtsProblems(ts);
-               rpmpsAppend(ps, RPMPROB_BADRELOCATE,
-                       rpmteNEVRA(p), rpmteKey(p),
-                       relocations[i].oldPath, NULL, NULL, 0);
-               ps = rpmpsFree(ps);
-           }
-           del =
-               strlen(relocations[i].newPath) - strlen(relocations[i].oldPath);
-
-           if (del > reldel)
-               reldel = del;
-       } else {
-           relocations[i].newPath = NULL;
-       }
-    }
-
-    sortRelocs(relocations, numRelocations);
 
     if (!_printed) {
        _printed = 1;
@@ -948,11 +865,11 @@ void rpmRelocateFileList(rpmts ts, rpmte p,
        }
     }
 
-    /* Add relocation values to the header */
-    if (numValid) {
-       addPrefixes(h, &validRelocs, relocations, numRelocations);
+    for (i = 0; i < numRelocations; i++) {
+       if (relocations[i].newPath == NULL) continue;
+       size_t len = strlen(relocations[i].newPath);
+       if (len > maxlen) maxlen = len;
     }
-    rpmtdFreeData(&validRelocs);
 
     headerGet(h, RPMTAG_BASENAMES, &bnames, HEADERGET_MINMEM);
     headerGet(h, RPMTAG_DIRINDEXES, &dindexes, HEADERGET_ALLOC);
@@ -979,7 +896,7 @@ void rpmRelocateFileList(rpmts ts, rpmte p,
        rpmFileTypes ft;
        int fnlen;
 
-       len = reldel +
+       size_t len = maxlen +
                strlen(dirNames[dirIndexes[i]]) + strlen(baseNames[i]) + 1;
        if (len >= fileAlloced) {
            fileAlloced = len * 2;
@@ -1099,7 +1016,7 @@ assert(fn != NULL);               /* XXX can't happen */
 
            if (relocations[j].oldPath == NULL) /* XXX can't happen */
                continue;
-           len = strcmp(relocations[j].oldPath, "/")
+           size_t len = strcmp(relocations[j].oldPath, "/")
                ? strlen(relocations[j].oldPath)
                : 0;
 
@@ -1139,11 +1056,6 @@ assert(fn != NULL);              /* XXX can't happen */
     rpmtdFreeData(&dindexes);
     rpmtdFreeData(&fmodes);
     free(fn);
-    for (i = 0; i < numRelocations; i++) {
-       free(relocations[i].oldPath);
-       free(relocations[i].newPath);
-    }
-    free(relocations);
 }
 
 rpmfi rpmfiFree(rpmfi fi)

diff --git a/lib/rpmte.c b/lib/rpmte.c
index 6c4c608..8a88020 100644 (file)
--- a/lib/rpmte.c
+++ b/lib/rpmte.c
@@ -86,14 +86,12 @@ void rpmteCleanDS(rpmte te)
  */
 static void delTE(rpmte p)
 {
-    rpmRelocation * r;
-
     if (p->relocs) {
-       for (r = p->relocs; (r->oldPath || r->newPath); r++) {
-           r->oldPath = _free(r->oldPath);
-           r->newPath = _free(r->newPath);
+       for (int i = 0; i < p->nrelocs; i++) {
+           free(p->relocs[i].oldPath);
+           free(p->relocs[i].newPath);
        }
-       p->relocs = _free(p->relocs);
+       free(p->relocs);
     }
 
     rpmteCleanDS(p);
@@ -130,12 +128,100 @@ static rpmfi getFI(rpmte p, rpmts ts, Header h)
     /* relocate stuff in header if necessary */
     if (rpmteType(p) == TR_ADDED) {
        if (!headerIsSource(h) && !headerIsEntry(h, RPMTAG_ORIGBASENAMES)) {
-           rpmRelocateFileList(ts, p, p->relocs, p->nrelocs, h);
+           rpmRelocateFileList(p, p->relocs, p->nrelocs, h);
        }
     }
     return rpmfiNew(ts, h, RPMTAG_BASENAMES, fiflags);
 }
 
+/* stupid bubble sort, but it's probably faster here */
+static void sortRelocs(rpmRelocation *relocations, int numRelocations)
+{
+    for (int i = 0; i < numRelocations; i++) {
+       int madeSwap = 0;
+       for (int j = 1; j < numRelocations; j++) {
+           rpmRelocation tmpReloc;
+           if (relocations[j - 1].oldPath == NULL || /* XXX can't happen */
+               relocations[j    ].oldPath == NULL || /* XXX can't happen */
+               strcmp(relocations[j - 1].oldPath, relocations[j].oldPath) <= 0)
+               continue;
+           /* LCL: ??? */
+           tmpReloc = relocations[j - 1];
+           relocations[j - 1] = relocations[j];
+           relocations[j] = tmpReloc;
+           madeSwap = 1;
+       }
+       if (!madeSwap) break;
+    }
+}
+
+static void buildRelocs(rpmts ts, rpmte p, Header h, rpmRelocation *relocs)
+{
+    int allowBadRelocate = (rpmtsFilterFlags(ts) & RPMPROB_FILTER_FORCERELOCATE);
+    int i;
+    struct rpmtd_s validRelocs;
+
+    for (rpmRelocation *r = relocs; r->oldPath || r->newPath; r++)
+       p->nrelocs++;
+
+    headerGet(h, RPMTAG_PREFIXES, &validRelocs, HEADERGET_MINMEM);
+    p->relocs = xmalloc(sizeof(*p->relocs) * (p->nrelocs+1));
+
+    /* Build sorted relocation list from raw relocations. */
+    for (i = 0; i < p->nrelocs; i++) {
+       char * t;
+
+       /*
+        * Default relocations (oldPath == NULL) are handled in the UI,
+        * not rpmlib.
+        */
+       if (relocs[i].oldPath == NULL) continue; /* XXX can't happen */
+
+       /* FIXME: Trailing /'s will confuse us greatly. Internal ones will 
+          too, but those are more trouble to fix up. :-( */
+       t = xstrdup(relocs[i].oldPath);
+       p->relocs[i].oldPath = (t[0] == '/' && t[1] == '\0')
+           ? t
+           : stripTrailingChar(t, '/');
+
+       /* An old path w/o a new path is valid, and indicates exclusion */
+       if (relocs[i].newPath) {
+           int valid = 0;
+           const char *validprefix;
+
+           t = xstrdup(relocs[i].newPath);
+           p->relocs[i].newPath = (t[0] == '/' && t[1] == '\0')
+               ? t
+               : stripTrailingChar(t, '/');
+
+               /* FIX:  relocations[i].oldPath == NULL */
+           /* Verify that the relocation's old path is in the header. */
+           rpmtdInit(&validRelocs);
+           while ((validprefix = rpmtdNextString(&validRelocs))) {
+               if (strcmp(validprefix, p->relocs[i].oldPath) == 0) {
+                   valid = 1;
+                   break;
+               }
+           }
+
+           if (!valid && !allowBadRelocate) {
+               rpmps ps = rpmtsProblems(ts);
+               rpmpsAppend(ps, RPMPROB_BADRELOCATE,
+                       rpmteNEVRA(p), rpmteKey(p),
+                       p->relocs[i].oldPath, NULL, NULL, 0);
+               ps = rpmpsFree(ps);
+           }
+       } else {
+           p->relocs[i].newPath = NULL;
+       }
+    }
+    p->relocs[i].oldPath = NULL;
+    p->relocs[i].newPath = NULL;
+    sortRelocs(p->relocs, p->nrelocs);
+    
+    rpmtdFreeData(&validRelocs);
+}
+
 /**
  * Initialize transaction element data from header.
  * @param ts           transaction set
@@ -180,21 +266,8 @@ static void addTE(rpmts ts, rpmte p, Header h,
 
     p->nrelocs = 0;
     p->relocs = NULL;
-    if (relocs != NULL) {
-       rpmRelocation * r;
-       int i;
-
-       for (r = relocs; r->oldPath || r->newPath; r++)
-           p->nrelocs++;
-       p->relocs = xmalloc((p->nrelocs + 1) * sizeof(*p->relocs));
-
-       for (i = 0, r = relocs; r->oldPath || r->newPath; i++, r++) {
-           p->relocs[i].oldPath = r->oldPath ? xstrdup(r->oldPath) : NULL;
-           p->relocs[i].newPath = r->newPath ? xstrdup(r->newPath) : NULL;
-       }
-       p->relocs[i].oldPath = NULL;
-       p->relocs[i].newPath = NULL;
-    }
+    if (relocs != NULL)
+       buildRelocs(ts, p, h, relocs);
 
     p->db_instance = headerGetInstance(h);
     p->key = key;

diff --git a/lib/rpmte_internal.h b/lib/rpmte_internal.h
index 3eb469a..abe0ea9 100644 (file)
--- a/lib/rpmte_internal.h
+++ b/lib/rpmte_internal.h
@@ -116,7 +116,7 @@ void rpmfsSetAction(rpmfs fs, unsigned int ix, rpmFileAction action);
 
 RPM_GNUC_INTERNAL
 /* XXX here for now... */
-void rpmRelocateFileList(rpmts ts, rpmte p, rpmRelocation *relocs, int numRelocations, Header h);
+void rpmRelocateFileList(rpmte p, rpmRelocation *relocs, int numRelocations, Header h);
 
 #endif /* _RPMTE_INTERNAL_H */