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;