Hi rpm Maintainers, I think I've found two bugs in the rpmstrPoolRehash() function:
1) IMHO there's an off-by-one in the for loop: pool->offs_size is the last used id, thus it should be "<=" instead of "<". 2) the function should to skip the "dummy" entries that are put at the end of each chunk. A chunk looks like this: foo\0bar\0...\0baz\0 ^ ^ ^ ^ ^ The dummy entry is there to make rpmstrPoolStrlen() work. Putting it in the hash is wrong. I've changed the code so that: - \0 is written to where the dummy entries point (not strictly needed as chunks are allocated with calloc, but nevertheless good style). - the rpmstrPoolRehash() loop checks if the string is of size zero (true for dummy entries). If that's the case it checks if the next string does not start after the \0, if that's also true it is a dummy entry. There are different (and easier) ways to fix this: - you can always put an empty string into the pool, it would always have id 1. This simplifies the dummy entry check to: if (i != 1 && str[0] == 0) - you could get rid of the dummy entries and remove the rpmstrPoolStrlen() function. It's only used 5 times in the code and calling strlen() on the returned string does not cost much. Hmm, I just see that the code in rpmfc.c also loops over the ids including the dummy entries. Oh my. rpmfcApply() should at least ignore "" entries. And it seems to modify the string returned from the strpool. Oh my again. (But using strtol() to convert a "const char *" to a "char *" is clever.) Cheers, Michael. -- Michael Schroeder m...@suse.de SUSE LINUX Products GmbH, GF Jeff Hawn, HRB 16746 AG Nuernberg main(_){while(_=~getchar())putchar(~_-1/(~(_|32)/13*2-11)*13);}
--- rpmio/rpmstrpool.c.orig 2013-09-11 15:33:48.371571600 +0000 +++ rpmio/rpmstrpool.c 2013-09-11 16:20:56.106566595 +0000 @@ -219,8 +219,17 @@ static void rpmstrPoolRehash(rpmstrPool pool->hash = poolHashFree(pool->hash); pool->hash = poolHashCreate(sizehint); - for (int i = 1; i < pool->offs_size; i++) - poolHashAddEntry(pool, rpmstrPoolStr(pool, i), i); + for (int i = 1; i <= pool->offs_size; i++) { + /* this is a little bit tricky because we have to skip the dummy + * entries that are at the end of each chunk */ + const char * str = rpmstrPoolStr(pool, i); + if (str[0] == 0 && i < pool->offs_size) { + /* looks like a dummy entry, check if next str is in a different chunk */ + if (rpmstrPoolStr(pool, i + 1) != str + 1) + continue; + } + poolHashAddEntry(pool, str, i); + } } rpmstrPool rpmstrPoolCreate(void) @@ -308,7 +317,8 @@ static rpmsid rpmstrPoolPut(rpmstrPool p } chunk_used = pool->offs[pool->offs_size] - pool->chunks[pool->chunks_size]; - if (ssize + 1 > pool->chunk_allocated - chunk_used) { + /* +2: extra trailing zero + extra byte */ + if (ssize + 2 > pool->chunk_allocated - chunk_used) { /* check size of ->chunks */ pool->chunks_size += 1; if (pool->chunks_size >= pool->chunks_allocated) { @@ -318,11 +328,12 @@ static rpmsid rpmstrPoolPut(rpmstrPool p } /* Check if string is bigger than chunks */ - if (ssize > pool->chunk_allocated) { - pool->chunk_allocated = 2 * ssize; + if (ssize + 2 > pool->chunk_allocated) { + pool->chunk_allocated = 2 * ssize + 2; } /* Dummy entry for end of last string*/ + pool->offs[pool->offs_size][0] = 0; pool->offs_size += 1; pool->offs[pool->offs_size] = xcalloc(1, pool->chunk_allocated);
_______________________________________________ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint