[pacman-dev] [PATCH] Handle null pkgcache for db_populate, refactor hash

Pang Yan Han pangyanhan at gmail.com
Mon Feb 7 09:59:20 EST 2011


In sync_db_populate() and local_db_populate(), a NULL
db->pkgcache is not handled. This not only involves unnecessary
operations, but also potentially merge sorting
db->pkgcache->list which does not even exist in the case that
db->pkgcache is NULL.

This may lead to serious issues since alpm_list_msort calls
alpm_list_nth which will dereference invalid pointers.
Thus, db->pkgcache is checked during local/sync db_populate
calls.

For this purpose, PM_ERR_DB_PKGCACHE_NULL error is added to
_pmerrno_t and taken care of in alpm_strerror().

In addition, common code between _alpm_pkghash_add() and
_alpm_pkghash_add_sorted() is refactored out to
_alpm_pkghash_add_pkg(). The 2 functions are now wrappers
around _alpm_pkghash_add_pkg().

Signed-off-by: Pang Yan Han <pangyanhan at gmail.com>
---
Please give me advice on this as to whether it needs tweaking
or if it won't be accepted. Thanks!

 lib/libalpm/alpm.h     |    1 +
 lib/libalpm/be_local.c |    4 ++++
 lib/libalpm/be_sync.c  |    8 +++++++-
 lib/libalpm/db.c       |    1 +
 lib/libalpm/error.c    |    2 ++
 lib/libalpm/pkghash.c  |   46 +++++++++++++++-------------------------------
 lib/libalpm/pkghash.h  |    1 +
 7 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 7fec293..57c68bd 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -501,6 +501,7 @@ enum _pmerrno_t {
 	PM_ERR_DB_NOT_FOUND,
 	PM_ERR_DB_WRITE,
 	PM_ERR_DB_REMOVE,
+	PM_ERR_DB_PKGCACHE_NULL,
 	/* Servers */
 	PM_ERR_SERVER_BAD_URL,
 	PM_ERR_SERVER_NONE,
diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c
index c5b2498..c7980f9 100644
--- a/lib/libalpm/be_local.c
+++ b/lib/libalpm/be_local.c
@@ -393,6 +393,10 @@ static int local_db_populate(pmdb_t *db)
 
 	/* initialize hash at 50% full */
 	db->pkgcache = _alpm_pkghash_create(est_count * 2);
+	if(db->pkgcache == NULL){
+		closedir(dbdir);
+		RET_ERR(PM_ERR_DB_PKGCACHE_NULL, -1);
+	}
 
 	while((ent = readdir(dbdir)) != NULL) {
 		const char *name = ent->d_name;
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index 4ad045c..853ad30 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -234,6 +234,9 @@ static int sync_db_populate(pmdb_t *db)
 
 	/* initialize hash at 66% full */
 	db->pkgcache = _alpm_pkghash_create(est_count * 3 / 2);
+	if(db->pkgcache == NULL) {
+		RET_ERR(PM_ERR_DB_PKGCACHE_NULL, -1);
+	}
 
 	while(archive_read_next_header(archive, &entry) == ARCHIVE_OK) {
 		const struct stat *st;
@@ -348,6 +351,9 @@ static int sync_db_read(pmdb_t *db, struct archive *archive,
 	if(likely_pkg && strcmp(likely_pkg->name, pkgname) == 0) {
 		pkg = likely_pkg;
 	} else {
+		if(db->pkgcache == NULL) {
+			RET_ERR(PM_ERR_DB_PKGCACHE_NULL, -1);
+		}
 		pkg = _alpm_pkghash_find(db->pkgcache, pkgname);
 	}
 	if(pkg == NULL) {
@@ -459,10 +465,10 @@ pmdb_t *_alpm_db_register_sync(const char *treename)
 	_alpm_log(PM_LOG_DEBUG, "registering sync database '%s'\n", treename);
 
 	db = _alpm_db_new(treename, 0);
-	db->ops = &sync_db_ops;
 	if(db == NULL) {
 		RET_ERR(PM_ERR_DB_CREATE, NULL);
 	}
+	db->ops = &sync_db_ops;
 
 	handle->dbs_sync = alpm_list_add(handle->dbs_sync, db);
 	return(db);
diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c
index 02f8282..d43a16b 100644
--- a/lib/libalpm/db.c
+++ b/lib/libalpm/db.c
@@ -370,6 +370,7 @@ pmdb_t *_alpm_db_new(const char *treename, int is_local)
 	CALLOC(db, 1, sizeof(pmdb_t), RET_ERR(PM_ERR_MEMORY, NULL));
 	STRDUP(db->treename, treename, RET_ERR(PM_ERR_MEMORY, NULL));
 	db->is_local = is_local;
+	db->pkgcache_loaded = 0;
 
 	return(db);
 }
diff --git a/lib/libalpm/error.c b/lib/libalpm/error.c
index 2cc6685..5b48c89 100644
--- a/lib/libalpm/error.c
+++ b/lib/libalpm/error.c
@@ -81,6 +81,8 @@ const char SYMEXPORT *alpm_strerror(int err)
 			return _("could not update database");
 		case PM_ERR_DB_REMOVE:
 			return _("could not remove database entry");
+		case PM_ERR_DB_PKGCACHE_NULL:
+			return _("could not initialize database package cache");
 		/* Servers */
 		case PM_ERR_SERVER_BAD_URL:
 			return _("invalid url for server");
diff --git a/lib/libalpm/pkghash.c b/lib/libalpm/pkghash.c
index 5480527..7006888 100644
--- a/lib/libalpm/pkghash.c
+++ b/lib/libalpm/pkghash.c
@@ -150,6 +150,16 @@ static pmpkghash_t *rehash(pmpkghash_t *oldhash)
 
 pmpkghash_t *_alpm_pkghash_add(pmpkghash_t *hash, pmpkg_t *pkg)
 {
+	return(_alpm_pkghash_add_pkg(hash, pkg, 0));
+}
+
+pmpkghash_t *_alpm_pkghash_add_sorted(pmpkghash_t *hash, pmpkg_t *pkg)
+{
+	return(_alpm_pkghash_add_pkg(hash, pkg, 1));
+}
+
+pmpkghash_t *_alpm_pkghash_add_pkg(pmpkghash_t *hash, pmpkg_t *pkg, int sorted)
+{
 	alpm_list_t *ptr;
 	size_t position;
 
@@ -173,40 +183,13 @@ pmpkghash_t *_alpm_pkghash_add(pmpkghash_t *hash, pmpkg_t *pkg)
 	ptr->prev = ptr;
 
 	hash->hash_table[position] = ptr;
-	hash->list = alpm_list_join(hash->list, ptr);
-	hash->entries += 1;
-
-	return(hash);
-}
-
-pmpkghash_t *_alpm_pkghash_add_sorted(pmpkghash_t *hash, pmpkg_t *pkg)
-{
-	if(!hash) {
-		return(_alpm_pkghash_add(hash, pkg));
-	}
-
-	alpm_list_t *ptr;
-	size_t position;
-
-	if((hash->entries + 1) / MAX_HASH_LOAD > hash->buckets) {
-		hash = rehash(hash);
+	if(!sorted){
+		hash->list = alpm_list_join(hash->list, ptr);
+	}else{
+		hash->list = alpm_list_mmerge(hash->list, ptr, _alpm_pkg_cmp);
 	}
 
-	position = get_hash_position(pkg->name_hash, hash);
-
-	ptr = calloc(1, sizeof(alpm_list_t));
-	if(ptr == NULL) {
-		return(hash);
-	}
-
-	ptr->data = pkg;
-	ptr->next = NULL;
-	ptr->prev = ptr;
-
-	hash->hash_table[position] = ptr;
-	hash->list = alpm_list_mmerge(hash->list, ptr, _alpm_pkg_cmp);
 	hash->entries += 1;
-
 	return(hash);
 }
 
@@ -344,3 +327,4 @@ pmpkg_t *_alpm_pkghash_find(pmpkghash_t *hash, const char *name)
 
 	return(NULL);
 }
+/* vim: set ts=2 sw=2 noet: */
diff --git a/lib/libalpm/pkghash.h b/lib/libalpm/pkghash.h
index a6c1db7..d63c6f0 100644
--- a/lib/libalpm/pkghash.h
+++ b/lib/libalpm/pkghash.h
@@ -47,6 +47,7 @@ pmpkghash_t *_alpm_pkghash_create(size_t size);
 
 pmpkghash_t *_alpm_pkghash_add(pmpkghash_t *hash, pmpkg_t *pkg);
 pmpkghash_t *_alpm_pkghash_add_sorted(pmpkghash_t *hash, pmpkg_t *pkg);
+pmpkghash_t *_alpm_pkghash_add_pkg(pmpkghash_t *hash, pmpkg_t *pkg, int sorted);
 pmpkghash_t *_alpm_pkghash_remove(pmpkghash_t *hash, pmpkg_t *pkg, pmpkg_t **data);
 
 void _alpm_pkghash_free(pmpkghash_t *hash);
-- 
1.7.4



More information about the pacman-dev mailing list