commit da3a3a14e757ccd517e2eb2a3f0293ff48b3ff7f Author: Panu Matilainen 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;