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;