Refactor database path and fd handling

This returns db->path to be "protected", e.g., available for use inside
the library, and removes the need to go through _alpm_db_path(), which
no longer exists. This was added due to us needing to do lazy resolving
because of config parsing, where DBPath settings could come after repos
were already established. However, we have since changed this and
require dbpath to be specified when calling alpm_initialize(), so the
lazy logic is no longer required.

This also adds a db->fd item to the database struct; sync databases are
wired up so we always have a permanent file handle to the database
object that we can use as necessary.

Signed-off-by: Dan McGee <dan@archlinux.org>

9 files changed, 111 insertions, 104 deletions

diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
index b0077665..c0e08656 100644
--- a/lib/libalpm/add.c
+++ b/lib/libalpm/add.c
@@ -157,12 +157,12 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive,
 	if(strcmp(entryname, ".INSTALL") == 0) {
 		/* the install script goes inside the db */
 		snprintf(filename, PATH_MAX, "%s%s-%s/install",
-				_alpm_db_path(handle->db_local), newpkg->name, newpkg->version);
+				handle->db_local->path, newpkg->name, newpkg->version);
 		archive_entry_set_perm(entry, 0644);
 	} else if(strcmp(entryname, ".CHANGELOG") == 0) {
 		/* the changelog goes inside the db */
 		snprintf(filename, PATH_MAX, "%s%s-%s/changelog",
-				_alpm_db_path(handle->db_local), newpkg->name, newpkg->version);
+				handle->db_local->path, newpkg->name, newpkg->version);
 		archive_entry_set_perm(entry, 0644);
 	} else if(*entryname == '.') {
 		/* for now, ignore all files starting with '.' that haven't
diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c
index 39df15b0..05b28fa3 100644
--- a/lib/libalpm/be_local.c
+++ b/lib/libalpm/be_local.c
@@ -251,17 +251,17 @@ static struct pkg_operations local_pkg_ops = {
 static int checkdbdir(alpm_db_t *db)
 {
 	struct stat buf;
-	const char *path = _alpm_db_path(db);
 
-	if(stat(path, &buf) != 0) {
-		_alpm_log(db->handle, ALPM_LOG_DEBUG, "database dir '%s' does not exist, creating it\n",
-				path);
-		if(_alpm_makepath(path) != 0) {
+	if(stat(db->path, &buf) != 0) {
+		_alpm_log(db->handle, ALPM_LOG_DEBUG,
+				"database dir '%s' does not exist, creating it\n", db->path);
+		if(_alpm_makepath(db->path) != 0) {
 			RET_ERR(db->handle, ALPM_ERR_SYSTEM, -1);
 		}
 	} else if(!S_ISDIR(buf.st_mode)) {
-		_alpm_log(db->handle, ALPM_LOG_WARNING, _("removing invalid database: %s\n"), path);
-		if(unlink(path) != 0 || _alpm_makepath(path) != 0) {
+		_alpm_log(db->handle, ALPM_LOG_WARNING,
+				_("removing invalid database: %s\n"), db->path);
+		if(unlink(db->path) != 0 || _alpm_makepath(db->path) != 0) {
 			RET_ERR(db->handle, ALPM_ERR_SYSTEM, -1);
 		}
 	}
@@ -292,7 +292,6 @@ static int is_dir(const char *path, struct dirent *entry)
 static int local_db_validate(alpm_db_t *db)
 {
 	struct dirent *ent = NULL;
-	const char *dbpath;
 	DIR *dbdir;
 	int ret = -1;
 
@@ -303,11 +302,7 @@ static int local_db_validate(alpm_db_t *db)
 		return -1;
 	}
 
-	dbpath = _alpm_db_path(db);
-	if(dbpath == NULL) {
-		RET_ERR(db->handle, ALPM_ERR_DB_OPEN, -1);
-	}
-	dbdir = opendir(dbpath);
+	dbdir = opendir(db->path);
 	if(dbdir == NULL) {
 		if(errno == ENOENT) {
 			/* database dir doesn't exist yet */
@@ -330,11 +325,11 @@ static int local_db_validate(alpm_db_t *db)
 		if(strcmp(name, ".") == 0 || strcmp(name, "..") == 0) {
 			continue;
 		}
-		if(!is_dir(dbpath, ent)) {
+		if(!is_dir(db->path, ent)) {
 			continue;
 		}
 
-		snprintf(path, PATH_MAX, "%s%s/depends", dbpath, name);
+		snprintf(path, PATH_MAX, "%s%s/depends", db->path, name);
 		if(access(path, F_OK) == 0) {
 			/* we found a depends file- bail */
 			db->status &= ~DB_STATUS_VALID;
@@ -362,7 +357,6 @@ static int local_db_populate(alpm_db_t *db)
 	int count = 0;
 	struct stat buf;
 	struct dirent *ent = NULL;
-	const char *dbpath;
 	DIR *dbdir;
 
 	if(db->status & DB_STATUS_INVALID) {
@@ -370,13 +364,7 @@ static int local_db_populate(alpm_db_t *db)
 	}
 	/* note: DB_STATUS_MISSING is not fatal for local database */
 
-	dbpath = _alpm_db_path(db);
-	if(dbpath == NULL) {
-		/* pm_errno set in _alpm_db_path() */
-		return -1;
-	}
-
-	dbdir = opendir(dbpath);
+	dbdir = opendir(db->path);
 	if(dbdir == NULL) {
 		if(errno == ENOENT) {
 			/* no database existing yet is not an error */
@@ -423,7 +411,7 @@ static int local_db_populate(alpm_db_t *db)
 		if(strcmp(name, ".") == 0 || strcmp(name, "..") == 0) {
 			continue;
 		}
-		if(!is_dir(dbpath, ent)) {
+		if(!is_dir(db->path, ent)) {
 			continue;
 		}
 
@@ -483,13 +471,11 @@ char *_alpm_local_db_pkgpath(alpm_db_t *db, alpm_pkg_t *info,
 {
 	size_t len;
 	char *pkgpath;
-	const char *dbpath;
 
-	dbpath = _alpm_db_path(db);
-	len = strlen(dbpath) + strlen(info->name) + strlen(info->version) + 3;
+	len = strlen(db->path) + strlen(info->name) + strlen(info->version) + 3;
 	len += filename ? strlen(filename) : 0;
 	MALLOC(pkgpath, len, RET_ERR(db->handle, ALPM_ERR_MEMORY, NULL));
-	sprintf(pkgpath, "%s%s-%s/%s", dbpath, info->name, info->version,
+	sprintf(pkgpath, "%s%s-%s/%s", db->path, info->name, info->version,
 			filename ? filename : "");
 	return pkgpath;
 }
@@ -998,6 +984,7 @@ struct db_operations local_db_ops = {
 alpm_db_t *_alpm_db_register_local(alpm_handle_t *handle)
 {
 	alpm_db_t *db;
+	size_t pathsize;
 
 	_alpm_log(handle, ALPM_LOG_DEBUG, "registering local database\n");
 
@@ -1006,6 +993,11 @@ alpm_db_t *_alpm_db_register_local(alpm_handle_t *handle)
 		handle->pm_errno = ALPM_ERR_DB_CREATE;
 		return NULL;
 	}
+
+	pathsize = strlen(handle->dbpath) + strlen(db->treename) + 2;
+	CALLOC(db->path, 1, pathsize, RET_ERR(handle, ALPM_ERR_MEMORY, NULL));
+	sprintf(db->path, "%s%s/", handle->dbpath, db->treename);
+
 	db->ops = &local_db_ops;
 	db->handle = handle;
 
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index 9fadff54..1a9cc892 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -69,7 +69,6 @@ static char *get_sync_dir(alpm_handle_t *handle)
 static int sync_db_validate(alpm_db_t *db)
 {
 	alpm_siglevel_t level;
-	const char *dbpath;
 
 	if(db->status & DB_STATUS_VALID || db->status & DB_STATUS_MISSING) {
 		return 0;
@@ -79,14 +78,8 @@ static int sync_db_validate(alpm_db_t *db)
 		return -1;
 	}
 
-	dbpath = _alpm_db_path(db);
-	if(!dbpath) {
-		/* pm_errno set in _alpm_db_path() */
-		return -1;
-	}
-
 	/* we can skip any validation if the database doesn't exist */
-	if(_alpm_access(db->handle, NULL, dbpath, R_OK) != 0 && errno == ENOENT) {
+	if(_alpm_access(db->handle, NULL, db->path, R_OK) != 0 && errno == ENOENT) {
 		db->status &= ~DB_STATUS_EXISTS;
 		db->status |= DB_STATUS_MISSING;
 		_alpm_log(db->handle, ALPM_LOG_WARNING,
@@ -105,7 +98,7 @@ static int sync_db_validate(alpm_db_t *db)
 		do {
 			retry = 0;
 			alpm_siglist_t *siglist;
-			ret = _alpm_check_pgp_helper(db->handle, dbpath, NULL,
+			ret = _alpm_check_pgp_helper(db->handle, db->path, NULL,
 					level & ALPM_SIG_DATABASE_OPTIONAL, level & ALPM_SIG_DATABASE_MARGINAL_OK,
 					level & ALPM_SIG_DATABASE_UNKNOWN_OK, &siglist);
 			if(ret) {
@@ -224,7 +217,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
 
 		if(ret == 0 && (level & ALPM_SIG_DATABASE)) {
 			/* an existing sig file is no good at this point */
-			char *sigpath = _alpm_sigpath(handle, _alpm_db_path(db));
+			char *sigpath = _alpm_sigpath(handle, db->path);
 			if(!sigpath) {
 				ret = -1;
 				break;
@@ -269,6 +262,8 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
 
 	/* Cache needs to be rebuilt */
 	_alpm_db_free_pkgcache(db);
+	CLOSE(db->fd);
+	db->fd = -1;
 
 	/* clear all status flags regarding validity/existence */
 	db->status &= ~DB_STATUS_VALID;
@@ -281,6 +276,14 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
 		ret = -1;
 	}
 
+	/* open an fd to the database if the database exists */
+	if(!(db->status & DB_STATUS_MISSING)) {
+		OPEN(db->fd, db->path, O_RDONLY);
+		if(db->fd < 0) {
+			RET_ERR(handle, ALPM_ERR_DB_OPEN, -1);
+		}
+	}
+
 cleanup:
 
 	if(_alpm_handle_unlock(handle)) {
@@ -413,9 +416,8 @@ static size_t estimate_package_count(struct stat *st, struct archive *archive)
 
 static int sync_db_populate(alpm_db_t *db)
 {
-	const char *dbpath;
 	size_t est_count;
-	int count, fd;
+	int ret, count;
 	struct stat buf;
 	struct archive *archive;
 	struct archive_entry *entry;
@@ -427,15 +429,9 @@ static int sync_db_populate(alpm_db_t *db)
 	if(db->status & DB_STATUS_MISSING) {
 		RET_ERR(db->handle, ALPM_ERR_DB_NOT_FOUND, -1);
 	}
-	dbpath = _alpm_db_path(db);
-	if(!dbpath) {
-		/* pm_errno set in _alpm_db_path() */
-		return -1;
-	}
 
-	fd = _alpm_open_archive(db->handle, dbpath, &buf,
-			&archive, ALPM_ERR_DB_OPEN);
-	if(fd < 0) {
+	ret = _alpm_open_fd(db->handle, db->fd, &buf, &archive, ALPM_ERR_DB_OPEN);
+	if(ret < 0) {
 		return -1;
 	}
 	est_count = estimate_package_count(&buf, archive);
@@ -473,9 +469,6 @@ static int sync_db_populate(alpm_db_t *db)
 
 cleanup:
 	archive_read_finish(archive);
-	if(fd >= 0) {
-		CLOSE(fd);
-	}
 	return count;
 }
 
@@ -633,6 +626,7 @@ alpm_db_t *_alpm_db_register_sync(alpm_handle_t *handle, const char *treename,
 		alpm_siglevel_t level)
 {
 	alpm_db_t *db;
+	size_t pathsize;
 
 	_alpm_log(handle, ALPM_LOG_DEBUG, "registering sync database '%s'\n", treename);
 
@@ -646,12 +640,26 @@ alpm_db_t *_alpm_db_register_sync(alpm_handle_t *handle, const char *treename,
 	if(db == NULL) {
 		RET_ERR(handle, ALPM_ERR_DB_CREATE, NULL);
 	}
+
+	pathsize = strlen(handle->dbpath) + 5 + strlen(db->treename) + 4;
+	CALLOC(db->path, 1, pathsize, RET_ERR(handle, ALPM_ERR_MEMORY, NULL));
+	/* all sync DBs now reside in the sync/ subdir of the dbpath */
+	sprintf(db->path, "%ssync/%s.db", handle->dbpath, db->treename);
+
 	db->ops = &sync_db_ops;
 	db->handle = handle;
 	db->siglevel = level;
 
 	sync_db_validate(db);
 
+	/* open an fd to the database if the database exists */
+	if(!(db->status & DB_STATUS_MISSING)) {
+		OPEN(db->fd, db->path, O_RDONLY);
+		if(db->fd < 0) {
+			RET_ERR(handle, ALPM_ERR_DB_OPEN, NULL);
+		}
+	}
+
 	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 8bbdc903..8b9886e8 100644
--- a/lib/libalpm/db.c
+++ b/lib/libalpm/db.c
@@ -25,6 +25,8 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <errno.h>
+#include <unistd.h>
 #include <regex.h>
 
 /* libalpm */
@@ -303,6 +305,7 @@ alpm_db_t *_alpm_db_new(const char *treename, int is_local)
 	} else {
 		db->status &= ~DB_STATUS_LOCAL;
 	}
+	db->fd = -1;
 
 	return db;
 }
@@ -313,44 +316,16 @@ void _alpm_db_free(alpm_db_t *db)
 	_alpm_db_free_pkgcache(db);
 	/* cleanup server list */
 	FREELIST(db->servers);
-	FREE(db->_path);
+	if(db->fd >= 0) {
+		CLOSE(db->fd);
+	}
+	FREE(db->path);
 	FREE(db->treename);
 	FREE(db);
 
 	return;
 }
 
-const char *_alpm_db_path(alpm_db_t *db)
-{
-	if(!db) {
-		return NULL;
-	}
-	if(!db->_path) {
-		const char *dbpath;
-		size_t pathsize;
-
-		dbpath = db->handle->dbpath;
-		if(!dbpath) {
-			_alpm_log(db->handle, ALPM_LOG_ERROR, _("database path is undefined\n"));
-			RET_ERR(db->handle, ALPM_ERR_DB_OPEN, NULL);
-		}
-
-		if(db->status & DB_STATUS_LOCAL) {
-			pathsize = strlen(dbpath) + strlen(db->treename) + 2;
-			CALLOC(db->_path, 1, pathsize, RET_ERR(db->handle, ALPM_ERR_MEMORY, NULL));
-			sprintf(db->_path, "%s%s/", dbpath, db->treename);
-		} else {
-			pathsize = strlen(dbpath) + 5 + strlen(db->treename) + 4;
-			CALLOC(db->_path, 1, pathsize, RET_ERR(db->handle, ALPM_ERR_MEMORY, NULL));
-			/* all sync DBs now reside in the sync/ subdir of the dbpath */
-			sprintf(db->_path, "%ssync/%s.db", dbpath, db->treename);
-		}
-		_alpm_log(db->handle, ALPM_LOG_DEBUG, "database path for tree %s set to %s\n",
-				db->treename, db->_path);
-	}
-	return db->_path;
-}
-
 int _alpm_db_cmp(const void *d1, const void *d2)
 {
 	const alpm_db_t *db1 = d1;
diff --git a/lib/libalpm/db.h b/lib/libalpm/db.h
index 94659b77..635b7fca 100644
--- a/lib/libalpm/db.h
+++ b/lib/libalpm/db.h
@@ -64,8 +64,7 @@ struct db_operations {
 struct __alpm_db_t {
 	alpm_handle_t *handle;
 	char *treename;
-	/* do not access directly, use _alpm_db_path(db) for lazy access */
-	char *_path;
+	char *path;
 	alpm_pkghash_t *pkgcache;
 	alpm_list_t *grpcache;
 	alpm_list_t *servers;
@@ -73,13 +72,13 @@ struct __alpm_db_t {
 	/* flags determining validity, local, loaded caches, etc. */
 	enum _alpm_dbstatus_t status;
 	alpm_siglevel_t siglevel;
+	int fd;
 };
 
 
 /* db.c, database general calls */
 alpm_db_t *_alpm_db_new(const char *treename, int is_local);
 void _alpm_db_free(alpm_db_t *db);
-const char *_alpm_db_path(alpm_db_t *db);
 int _alpm_db_cmp(const void *d1, const void *d2);
 alpm_list_t *_alpm_db_search(alpm_db_t *db, const alpm_list_t *needles);
 alpm_db_t *_alpm_db_register_local(alpm_handle_t *handle);
diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c
index 018b1cca..ccd0ebbd 100644
--- a/lib/libalpm/signing.c
+++ b/lib/libalpm/signing.c
@@ -893,7 +893,7 @@ int SYMEXPORT alpm_db_check_pgp_signature(alpm_db_t *db,
 	ASSERT(siglist != NULL, RET_ERR(db->handle, ALPM_ERR_WRONG_ARGS, -1));
 	db->handle->pm_errno = 0;
 
-	return _alpm_gpgme_checksig(db->handle, _alpm_db_path(db), NULL, siglist);
+	return _alpm_gpgme_checksig(db->handle, db->path, NULL, siglist);
 }
 
 /**
diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
index a392c773..f915db0d 100644
--- a/lib/libalpm/util.c
+++ b/lib/libalpm/util.c
@@ -230,7 +230,41 @@ size_t _alpm_strip_newline(char *str)
 int _alpm_open_archive(alpm_handle_t *handle, const char *path,
 		struct stat *buf, struct archive **archive, alpm_errno_t error)
 {
-	int fd;
+	int fd, open_fd;
+
+	_alpm_log(handle, ALPM_LOG_DEBUG, "opening archive %s\n", path);
+	OPEN(fd, path, O_RDONLY);
+	if(fd < 0) {
+		_alpm_log(handle, ALPM_LOG_ERROR,
+				_("could not open file %s: %s\n"), path, strerror(errno));
+		RET_ERR(handle, error, -1);
+	}
+
+	open_fd = _alpm_open_fd(handle, fd, buf, archive, error);
+	if(open_fd < 0) {
+		CLOSE(fd);
+		RET_ERR(handle, error, -1);
+	}
+
+	return open_fd;
+}
+
+/** Open an archive for reading and perform the necessary boilerplate.
+ * This takes care of creating the libarchive 'archive' struct, setting up
+ * compression and format options, opening a file descriptor, setting up the
+ * buffer size, and performing a stat on the path once opened.
+ * On error, no file descriptor is opened, and the archive pointer returned
+ * will be set to NULL.
+ * @param handle the context handle
+ * @param fd an existing open file descriptor, will never be closed
+ * @param buf space for a stat buffer for the given path
+ * @param archive pointer to place the created archive object
+ * @param error error code to set on failure to open archive
+ * @return -1 on failure, >=0 file descriptor on success
+ */
+int _alpm_open_fd(alpm_handle_t *handle, int fd,
+		struct stat *buf, struct archive **archive, alpm_errno_t error)
+{
 	size_t bufsize = ALPM_BUFFER_SIZE;
 	errno = 0;
 
@@ -241,17 +275,9 @@ int _alpm_open_archive(alpm_handle_t *handle, const char *path,
 	archive_read_support_compression_all(*archive);
 	archive_read_support_format_all(*archive);
 
-	_alpm_log(handle, ALPM_LOG_DEBUG, "opening archive %s\n", path);
-	OPEN(fd, path, O_RDONLY);
-	if(fd < 0) {
-		_alpm_log(handle, ALPM_LOG_ERROR,
-				_("could not open file %s: %s\n"), path, strerror(errno));
-		goto error;
-	}
-
 	if(fstat(fd, buf) != 0) {
 		_alpm_log(handle, ALPM_LOG_ERROR,
-				_("could not stat file %s: %s\n"), path, strerror(errno));
+				_("could not stat archive: %s\n"), strerror(errno));
 		goto error;
 	}
 #ifdef HAVE_STRUCT_STAT_ST_BLKSIZE
@@ -259,10 +285,11 @@ int _alpm_open_archive(alpm_handle_t *handle, const char *path,
 		bufsize = buf->st_blksize;
 	}
 #endif
+	lseek(fd, 0, SEEK_SET);
 
 	if(archive_read_open_fd(*archive, fd, bufsize) != ARCHIVE_OK) {
-		_alpm_log(handle, ALPM_LOG_ERROR, _("could not open file %s: %s\n"),
-				path, archive_error_string(*archive));
+		_alpm_log(handle, ALPM_LOG_ERROR, _("could not open archive: %s\n"),
+				archive_error_string(*archive));
 		goto error;
 	}
 
@@ -271,9 +298,6 @@ int _alpm_open_archive(alpm_handle_t *handle, const char *path,
 error:
 	archive_read_finish(*archive);
 	*archive = NULL;
-	if(fd >= 0) {
-		CLOSE(fd);
-	}
 	RET_ERR(handle, error, -1);
 }
 
diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h
index e35e35b9..b37007f7 100644
--- a/lib/libalpm/util.h
+++ b/lib/libalpm/util.h
@@ -112,6 +112,8 @@ size_t _alpm_strip_newline(char *str);
 
 int _alpm_open_archive(alpm_handle_t *handle, const char *path,
 		struct stat *buf, struct archive **archive, alpm_errno_t error);
+int _alpm_open_fd(alpm_handle_t *handle, int fd,
+		struct stat *buf, struct archive **archive, alpm_errno_t error);
 int _alpm_unpack_single(alpm_handle_t *handle, const char *archive,
 		const char *prefix, const char *filename);
 int _alpm_unpack(alpm_handle_t *handle, const char *archive, const char *prefix,
diff --git a/src/pacman/util.c b/src/pacman/util.c
index be8fc308..db3e4e6c 100644
--- a/src/pacman/util.c
+++ b/src/pacman/util.c
@@ -124,6 +124,13 @@ int check_syncdbs(size_t need_repos, int check_valid)
 						alpm_db_get_name(db), alpm_strerror(alpm_errno(config->handle)));
 				ret = 1;
 			}
+			/* also check that we can actually load data from it */
+			alpm_db_get_pkgcache(db);
+			if(alpm_errno(config->handle)) {
+				pm_printf(ALPM_LOG_ERROR, _("database '%s' is not valid (%s)\n"),
+						alpm_db_get_name(db), alpm_strerror(alpm_errno(config->handle)));
+				ret = 1;
+			}
 		}
 	}
 	return ret;