[pacman-dev] [PATCH 1/3] Give sensible feedback when a repo has no configured servers

[Dan McGee]

This fixes FS#14899. When running an -Sp operation without servers
configured for a repository, we would segfault, so add an assert to the
backend method returning the first server preventing a null pointer
dereference.

In addition, add a new error code to libalpm that indicates we have no
servers configured for a repository. This makes -Sy and -S <package>
operations fail gracefully and helpfully when a repo is set up with no
servers, as the default mirrorlist in Arch is provided this way.

Signed-off-by: Dan McGee <dan at archlinux.org>
---
 lib/libalpm/alpm.h  |    1 +
 lib/libalpm/db.c    |    1 +
 lib/libalpm/dload.c |    2 ++
 lib/libalpm/error.c |    2 ++
 lib/libalpm/sync.c  |    4 +++-
 src/pacman/sync.c   |   10 +++++++++-
 6 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 5cf1e94..1b67f8a 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -480,6 +480,7 @@ enum _pmerrno_t {
 	PM_ERR_DB_REMOVE,
 	/* Servers */
 	PM_ERR_SERVER_BAD_URL,
+	PM_ERR_SERVER_NONE,
 	/* Transactions */
 	PM_ERR_TRANS_NOT_NULL,
 	PM_ERR_TRANS_NULL,
diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c
index 561967c..6749ab1 100644
--- a/lib/libalpm/db.c
+++ b/lib/libalpm/db.c
@@ -232,6 +232,7 @@ const char SYMEXPORT *alpm_db_get_url(const pmdb_t *db)
 	/* Sanity checks */
 	ASSERT(handle != NULL, return(NULL));
 	ASSERT(db != NULL, return(NULL));
+	ASSERT(db->servers != NULL, return(NULL));
 
 	url = (char*)db->servers->data;
 
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 5b0a691..aec0b5c 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -388,6 +388,8 @@ int _alpm_download_single_file(const char *filename,
 	alpm_list_t *i;
 	int ret = -1;
 
+	ASSERT(servers != NULL, RET_ERR(PM_ERR_SERVER_NONE, -1));
+
 	for(i = servers; i; i = i->next) {
 		const char *server = i->data;
 		char *fileurl = NULL;
diff --git a/lib/libalpm/error.c b/lib/libalpm/error.c
index fdff9ed..47e254e 100644
--- a/lib/libalpm/error.c
+++ b/lib/libalpm/error.c
@@ -84,6 +84,8 @@ const char SYMEXPORT *alpm_strerror(int err)
 		/* Servers */
 		case PM_ERR_SERVER_BAD_URL:
 			return _("invalid url for server");
+		case PM_ERR_SERVER_NONE:
+			return _("no servers configured for repository");
 		/* Transactions */
 		case PM_ERR_TRANS_NOT_NULL:
 			return _("transaction already initialized");
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index abda147..519066c 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -777,7 +777,9 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
 			if(_alpm_download_files(files, current->servers, cachedir)) {
 				_alpm_log(PM_LOG_WARNING, _("failed to retrieve some files from %s\n"),
 						current->treename);
-				pm_errno = PM_ERR_RETRIEVE;
+				if(pm_errno == 0) {
+					pm_errno = PM_ERR_RETRIEVE;
+				}
 				goto error;
 			}
 			FREELIST(files);
diff --git a/src/pacman/sync.c b/src/pacman/sync.c
index 68fb81a..2e57b01 100644
--- a/src/pacman/sync.c
+++ b/src/pacman/sync.c
@@ -675,7 +675,15 @@ static int sync_trans(alpm_list_t *targets)
 		for(i = packages; i; i = alpm_list_next(i)) {
 			pmpkg_t *pkg = alpm_list_getdata(i);
 			pmdb_t *db = alpm_pkg_get_db(pkg);
-			printf("%s/%s\n", alpm_db_get_url(db), alpm_pkg_get_filename(pkg));
+			const char *dburl = alpm_db_get_url(db);
+			if(dburl) {
+				printf("%s/%s\n", dburl, alpm_pkg_get_filename(pkg));
+			} else {
+				/* can't use WARNING here, we don't show warnings in -Sp... */
+				pm_fprintf(stderr, PM_LOG_ERROR, _("no database for package: %s\n"),
+						alpm_pkg_get_name(pkg));
+			}
+
 		}
 		/* we are done */
 		goto cleanup;
-- 
1.6.3.2