michal-grzedzicki / rpms / rpm

Forked from rpms/rpm 5 months ago
Clone
Blob Blame History Raw
commit da3a3a14e757ccd517e2eb2a3f0293ff48b3ff7f
Author: Panu Matilainen <pmatilai@redhat.com>
Date:   Wed Feb 5 13:36:31 2014 +0200

    A boolean flag is not sufficient to cover three different states...
    
    - HEADERFLAG_SORTED flag bit has been used to track the header
      sort state since beginning of times, but the header sort actually
      has three different states: truly unsorted (such as after additions),
      sorted by index achieved with headerSort(), or sorted by offset which
      is creatively called headerUnsort() in the API.
    
      There's exactly one place that requires offset sorting, namely
      headerExport(). It does call headerUnsort() explicitly, but
      headerUnsort() would not do anything unless the header was in
      index-sorted state. This is quite often the case, but data addition
      would unset the sorted-flag, and causing headerUnsort() not to
      do anything at all when it most certainly should have. That incorrect
      sorting caused headerExport() calculations to be wildly off.
    
      This ancient bug was unearthed by the innocent looking
      commit eae671556470136c37951f53ca966cd1bd09aac4: prior to that,
      in half the cases headerSizeof() used to be called first, causing
      index-sorting to take place, after which the headerUnsort() in
      headerExport() did the right thing. In the other two cases
      headerSizeof() was called afterwards, and the headerExport() would
      be garbage in cases where new data had been just added, such as package
      installation. With headerExport() eliminating the need to call
      headerSizeof() all the cases became broken as the header would
      seem much bigger than it is due to the bad sort order and cause
      cases like RhBug:953719 where a largish header suddenly appears
      to be oversized, causing mysterious install failure. Prior to the
      commit (which in itself IS innocent) the issue would've mainly
      manifested in cases where the header needs to be rewritten, which
      only happens when files are replaced and the header is rewritten for that.

diff --git a/lib/header.c b/lib/header.c
index ceffb32..f78ba78 100644
--- a/lib/header.c
+++ b/lib/header.c
@@ -68,8 +68,13 @@ static const int typeSizes[16] =  {
     0
 };
 
+enum headerSorted_e {
+    HEADERSORT_NONE	= 0,	/* Not sorted */
+    HEADERSORT_OFFSET	= 1,	/* Sorted by offset (on-disk format) */
+    HEADERSORT_INDEX	= 2,	/* Sorted by index  */
+};
+
 enum headerFlags_e {
-    HEADERFLAG_SORTED    = (1 << 0), /*!< Are header entries sorted? */
     HEADERFLAG_ALLOCATED = (1 << 1), /*!< Is 1st header region allocated? */
     HEADERFLAG_LEGACY    = (1 << 2), /*!< Header came from legacy source? */
     HEADERFLAG_DEBUG     = (1 << 3), /*!< Debug this header? */
@@ -87,6 +92,7 @@ struct headerToken_s {
     int indexAlloced;		/*!< Allocated size of tag array. */
     unsigned int instance;	/*!< Rpmdb instance (offset) */
     headerFlags flags;
+    int sorted;			/*!< Current sort method */
     int nrefs;			/*!< Reference count. */
 };
 
@@ -169,7 +175,7 @@ static Header headerCreate(void *blob, unsigned int pvlen, int32_t indexLen)
 	h->indexUsed = 0;
     }
     h->instance = 0;
-    h->flags |= HEADERFLAG_SORTED;
+    h->sorted = HEADERSORT_NONE;
 
     h->index = (h->indexAlloced
 	? xcalloc(h->indexAlloced, sizeof(*h->index))
@@ -219,9 +225,9 @@ static int indexCmp(const void * avp, const void * bvp)
 
 void headerSort(Header h)
 {
-    if (!(h->flags & HEADERFLAG_SORTED)) {
+    if (h->sorted != HEADERSORT_INDEX) {
 	qsort(h->index, h->indexUsed, sizeof(*h->index), indexCmp);
-	h->flags |= HEADERFLAG_SORTED;
+	h->sorted = HEADERSORT_INDEX;
     }
 }
 
@@ -242,9 +248,9 @@ static int offsetCmp(const void * avp, const void * bvp)
 
 void headerUnsort(Header h)
 {
-    if (h->flags & HEADERFLAG_SORTED) {
+    if (h->sorted != HEADERSORT_OFFSET) {
 	qsort(h->index, h->indexUsed, sizeof(*h->index), offsetCmp);
-	h->flags &= ~HEADERFLAG_SORTED;
+	h->sorted = HEADERSORT_OFFSET;
     }
 }
 
@@ -721,7 +727,8 @@ indexEntry findEntry(Header h, rpmTagVal tag, rpm_tagtype_t type)
     struct indexEntry_s key;
 
     if (h == NULL) return NULL;
-    if (!(h->flags & HEADERFLAG_SORTED)) headerSort(h);
+    if (h->sorted != HEADERSORT_INDEX)
+	headerSort(h);
 
     key.info.tag = tag;
 
@@ -907,7 +914,8 @@ Header headerImport(void * blob, unsigned int bsize, headerImportFlags flags)
 	    goto errxit;
     }
 
-    h->flags &= ~HEADERFLAG_SORTED;
+    /* Force sorting, dribble lookups can cause early sort on partial header */
+    h->sorted = HEADERSORT_NONE;
     headerSort(h);
     h->flags |= HEADERFLAG_ALLOCATED;
 
@@ -1439,7 +1447,7 @@ static int intAddEntry(Header h, rpmtd td)
     entry->length = length;
 
     if (h->indexUsed > 0 && td->tag < h->index[h->indexUsed-1].info.tag)
-	h->flags &= ~HEADERFLAG_SORTED;
+	h->sorted = HEADERSORT_NONE;
     h->indexUsed++;
 
     return 1;