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
