[pacman-dev] [PATCH 1/6] Use standard errno codes in return from _alpm_archive_fgets

Dan McGee dan at archlinux.org
Fri Jun 3 18:40:57 EDT 2011


This allows us to not require the context (e.g. handle) when calling
this function. Also beef up the checks in the two callers of this
function to bail if the last return code is not ARCHIVE_EOF, which is
the expected value.

This requires a change to one of the pactest return codes and the
overall result of the test, but results in a much safer operating
condition whereby invalid database entries will stop the operation.

Signed-off-by: Dan McGee <dan at archlinux.org>
---
 lib/libalpm/be_package.c      |   10 +++++++---
 lib/libalpm/be_sync.c         |   22 ++++++++++++++++++----
 lib/libalpm/util.c            |    7 +++----
 test/pacman/pactest.py        |    1 +
 test/pacman/tests/smoke002.py |   12 +++++-------
 5 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c
index c70d1e9..c89365a 100644
--- a/lib/libalpm/be_package.c
+++ b/lib/libalpm/be_package.c
@@ -137,13 +137,13 @@ static struct pkg_operations *get_file_pkg_ops(void)
  * @param archive the archive to read from, pointed at the .PKGINFO entry
  * @param newpkg an empty pmpkg_t struct to fill with package info
  *
- * @return 0 on success, 1 on error
+ * @return 0 on success, -1 on error
  */
 static int parse_descfile(struct archive *a, pmpkg_t *newpkg)
 {
 	char *ptr = NULL;
 	char *key = NULL;
-	int linenum = 0;
+	int ret, linenum = 0;
 	struct archive_read_buffer buf;
 
 	memset(&buf, 0, sizeof(buf));
@@ -151,7 +151,7 @@ static int parse_descfile(struct archive *a, pmpkg_t *newpkg)
 	buf.max_line_size = 512 * 1024;
 
 	/* loop until we reach EOF or other error */
-	while(_alpm_archive_fgets(a, &buf) == ARCHIVE_OK) {
+	while((ret = _alpm_archive_fgets(a, &buf)) == ARCHIVE_OK) {
 		char *line = _alpm_strtrim(buf.line);
 
 		linenum++;
@@ -215,6 +215,10 @@ static int parse_descfile(struct archive *a, pmpkg_t *newpkg)
 		}
 		line[0] = '\0';
 	}
+	if(ret != ARCHIVE_EOF) {
+		_alpm_log(PM_LOG_DEBUG, "error parsing package descfile\n");
+		return -1;
+	}
 
 	return 0;
 }
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index 953e4dc..aeba86f 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -294,6 +294,7 @@ static int sync_db_populate(pmdb_t *db)
 				_alpm_log(PM_LOG_ERROR, _("invalid name for database entry '%s'\n"),
 						name);
 				_alpm_pkg_free(pkg);
+				pkg = NULL;
 				continue;
 			}
 
@@ -301,6 +302,7 @@ static int sync_db_populate(pmdb_t *db)
 			if(_alpm_pkghash_find(db->pkgcache, pkg->name)) {
 				_alpm_log(PM_LOG_ERROR, _("duplicated database entry '%s'\n"), pkg->name);
 				_alpm_pkg_free(pkg);
+				pkg = NULL;
 				continue;
 			}
 
@@ -316,7 +318,14 @@ static int sync_db_populate(pmdb_t *db)
 			count++;
 		} else {
 			/* we have desc, depends or deltas - parse it */
-			sync_db_read(db, archive, entry, pkg);
+			if(sync_db_read(db, archive, entry, pkg) != 0) {
+				_alpm_log(PM_LOG_ERROR,
+						_("could not parse package '%s' description file from db '%s'\n"),
+						pkg->name, db->treename);
+				_alpm_pkg_free(pkg);
+				pkg = NULL;
+				continue;
+			}
 		}
 	}
 
@@ -401,7 +410,8 @@ static int sync_db_read(pmdb_t *db, struct archive *archive,
 
 	if(strcmp(filename, "desc") == 0 || strcmp(filename, "depends") == 0
 			|| strcmp(filename, "deltas") == 0) {
-		while(_alpm_archive_fgets(archive, &buf) == ARCHIVE_OK) {
+		int ret;
+		while((ret = _alpm_archive_fgets(archive, &buf)) == ARCHIVE_OK) {
 			char *line = _alpm_strtrim(buf.line);
 
 			if(strcmp(line, "%NAME%") == 0) {
@@ -478,6 +488,9 @@ static int sync_db_read(pmdb_t *db, struct archive *archive,
 				}
 			}
 		}
+		if(ret != ARCHIVE_EOF) {
+			goto error;
+		}
 	} else if(strcmp(filename, "files") == 0) {
 		/* currently do nothing with this file */
 	} else {
@@ -485,10 +498,11 @@ static int sync_db_read(pmdb_t *db, struct archive *archive,
 		_alpm_log(PM_LOG_DEBUG, "unknown database file: %s\n", filename);
 	}
 
+	return 0;
+
 error:
 	FREE(pkgname);
-	/* TODO: return 0 always? */
-	return 0;
+	return -1;
 }
 
 static int sync_db_version(pmdb_t UNUSED *db)
diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
index e25e19d..5821bf9 100644
--- a/lib/libalpm/util.c
+++ b/lib/libalpm/util.c
@@ -771,20 +771,19 @@ int _alpm_archive_fgets(struct archive *a, struct archive_read_buffer *b)
 		/* allocate our buffer, or ensure our existing one is big enough */
 		if(!b->line) {
 			/* set the initial buffer to the read block_size */
-			CALLOC(b->line, b->block_size + 1, sizeof(char),
-					RET_ERR(PM_ERR_MEMORY, -1));
+			CALLOC(b->line, b->block_size + 1, sizeof(char), return ENOMEM);
 			b->line_size = b->block_size + 1;
 			b->line_offset = b->line;
 		} else {
 			size_t needed = (size_t)((b->line_offset - b->line)
 					+ (i - b->block_offset) + 1);
 			if(needed > b->max_line_size) {
-				RET_ERR(PM_ERR_MEMORY, -1);
+				return ERANGE;
 			}
 			if(needed > b->line_size) {
 				/* need to realloc + copy data to fit total length */
 				char *new;
-				CALLOC(new, needed, sizeof(char), RET_ERR(PM_ERR_MEMORY, -1));
+				CALLOC(new, needed, sizeof(char), return ENOMEM);
 				memcpy(new, b->line, b->line_size);
 				b->line_size = needed;
 				b->line_offset = new + (b->line_offset - b->line);
diff --git a/test/pacman/pactest.py b/test/pacman/pactest.py
index 64d5651..77f87da 100755
--- a/test/pacman/pactest.py
+++ b/test/pacman/pactest.py
@@ -114,6 +114,7 @@ def createOptParser():
     env.results()
 
     if env.failed > 0:
+        print "pacman testing root saved: %s" % root_path
         sys.exit(1)
 
     if not opts.keeproot:
diff --git a/test/pacman/tests/smoke002.py b/test/pacman/tests/smoke002.py
index 44f2d0e..8ff5cab 100644
--- a/test/pacman/tests/smoke002.py
+++ b/test/pacman/tests/smoke002.py
@@ -10,10 +10,8 @@
 
 self.args = "-U %s %s" % (p1.filename(), p2.filename())
 
-# Note that the current cutoff on line length is 512K, so the first package
-# will succeed while the second one will fail to record the description.
-self.addrule("PACMAN_RETCODE=0")
-self.addrule("PKG_EXIST=pkg1")
-self.addrule("PKG_DESC=pkg1|%s" % p1.desc)
-self.addrule("PKG_EXIST=pkg1")
-self.addrule("!PKG_DESC=pkg1|%s" % p2.desc)
+# We error out when fed a package with an invalid description; the second one
+# fits the bill in this case as the desc is > 512K
+self.addrule("PACMAN_RETCODE=1")
+self.addrule("!PKG_EXIST=pkg1")
+self.addrule("!PKG_EXIST=pkg1")
-- 
1.7.5.2



More information about the pacman-dev mailing list