Update the question callback

Much like with events, instead of using a bunch of void* arguments for
all questions, we now send one pointer to an alpm_question_t union.
This contains the type of question that was triggered.

With this information, a question-specific struct can be accessed in
order to get additional arguments.

Signed-off-by: Olivier Brunel <jjk@jjacky.com>
Signed-off-by: Allan McRae <allan@archlinux.org>

lib/libalpm/alpm.h

@@ -567,23 +567,119 @@ typedef struct _alpm_event_pacorig_created_t {
 typedef void (*alpm_cb_event)(alpm_event_t *);
 
 /**
- * Questions.
+ * Type of questions.
  * Unlike the events or progress enumerations, this enum has bitmask values
  * so a frontend can use a bitmask map to supply preselected answers to the
  * different types of questions.
  */
-typedef enum _alpm_question_t {
+typedef enum _alpm_question_type_t {
-	ALPM_QUESTION_INSTALL_IGNOREPKG = 1,
+	ALPM_QUESTION_INSTALL_IGNOREPKG = (1 << 0),
 	ALPM_QUESTION_REPLACE_PKG = (1 << 1),
 	ALPM_QUESTION_CONFLICT_PKG = (1 << 2),
 	ALPM_QUESTION_CORRUPTED_PKG = (1 << 3),
 	ALPM_QUESTION_REMOVE_PKGS = (1 << 4),
 	ALPM_QUESTION_SELECT_PROVIDER = (1 << 5),
 	ALPM_QUESTION_IMPORT_KEY = (1 << 6)
+} alpm_question_type_t;
+
+typedef struct _alpm_question_any_t {
+	/** Type of question. */
+	alpm_question_type_t type;
+	/** Answer. */
+	int answer;
+} alpm_question_any_t;
+
+typedef struct _alpm_question_install_ignorepkg_t {
+	/** Type of question. */
+	alpm_question_type_t type;
+	/** Answer: whether or not to install pkg anyway. */
+	int install;
+	/* Package in IgnorePkg/IgnoreGroup. */
+	alpm_pkg_t *pkg;
+} alpm_question_install_ignorepkg_t;
+
+typedef struct _alpm_question_replace_t {
+	/** Type of question. */
+	alpm_question_type_t type;
+	/** Answer: whether or not to replace oldpkg with newpkg. */
+	int replace;
+	/* Package to be replaced. */
+	alpm_pkg_t *oldpkg;
+	/* Package to replace with. */
+	alpm_pkg_t *newpkg;
+	/* DB of newpkg */
+	alpm_db_t *newdb;
+} alpm_question_replace_t;
+
+typedef struct _alpm_question_conflict_t {
+	/** Type of question. */
+	alpm_question_type_t type;
+	/** Answer: whether or not to remove conflict->package2. */
+	int remove;
+	/** Conflict info. */
+	alpm_conflict_t *conflict;
+} alpm_question_conflict_t;
+
+typedef struct _alpm_question_corrupted_t {
+	/** Type of question. */
+	alpm_question_type_t type;
+	/** Answer: whether or not to remove filepath. */
+	int remove;
+	/** Filename to remove */
+	const char *filepath;
+	/** Error code indicating the reason for package invalidity */
+	alpm_errno_t reason;
+} alpm_question_corrupted_t;
+
+typedef struct _alpm_question_remove_pkgs_t {
+	/** Type of question. */
+	alpm_question_type_t type;
+	/** Answer: whether or not to skip packages. */
+	int skip;
+	/** List of alpm_pkg_t* with unresolved dependencies. */
+	alpm_list_t *packages;
+} alpm_question_remove_pkgs_t;
+
+typedef struct _alpm_question_select_provider_t {
+	/** Type of question. */
+	alpm_question_type_t type;
+	/** Answer: which provider to use (index from providers). */
+	int use_index;
+	/** List of alpm_pkg_t* as possible providers. */
+	alpm_list_t *providers;
+	/** What providers provide for. */
+	alpm_depend_t *depend;
+} alpm_question_select_provider_t;
+
+typedef struct _alpm_question_import_key_t {
+	/** Type of question. */
+	alpm_question_type_t type;
+	/** Answer: whether or not to import key. */
+	int import;
+	/** The key to import. */
+	alpm_pgpkey_t *key;
+} alpm_question_import_key_t;
+
+/**
+ * Questions.
+ * This is an union passed to the callback, that allows the frontend to know
+ * which type of question was triggered (via type). It is then possible to
+ * typecast the pointer to the right structure, or use the union field, in order
+ * to access question-specific data. */
+typedef union _alpm_question_t {
+	alpm_question_type_t type;
+	alpm_question_any_t any;
+	alpm_question_install_ignorepkg_t install_ignorepkg;
+	alpm_question_replace_t replace;
+	alpm_question_conflict_t conflict;
+	alpm_question_corrupted_t corrupted;
+	alpm_question_remove_pkgs_t remove_pkgs;
+	alpm_question_select_provider_t select_provider;
+	alpm_question_import_key_t import_key;
 } alpm_question_t;
 
 /** Question callback */
-typedef void (*alpm_cb_question)(alpm_question_t, void *, void *, void *, int *);
+typedef void (*alpm_cb_question)(alpm_question_t *);
 
 /** Progress */
 typedef enum _alpm_progress_t {

lib/libalpm/deps.c

@@ -640,15 +640,18 @@ static alpm_pkg_t *resolvedep(alpm_handle_t *handle, alpm_depend_t *dep,
 		if(pkg && _alpm_depcmp_literal(pkg, dep)
 				&& !alpm_pkg_find(excluding, pkg->name)) {
 			if(alpm_pkg_should_ignore(handle, pkg)) {
-				int install = 0;
+				alpm_question_install_ignorepkg_t question = {
+					.type = ALPM_QUESTION_INSTALL_IGNOREPKG,
+					.install = 0,
+					.pkg = pkg
+				};
 				if(prompt) {
-					QUESTION(handle, ALPM_QUESTION_INSTALL_IGNOREPKG, pkg,
+					QUESTION(handle, &question);
-							 NULL, NULL, &install);
 				} else {
 					_alpm_log(handle, ALPM_LOG_WARNING, _("ignoring package %s-%s\n"),
 							pkg->name, pkg->version);
 				}
-				if(!install) {
+				if(!question.install) {
 					ignored = 1;
 					continue;
 				}
@@ -669,15 +672,18 @@ static alpm_pkg_t *resolvedep(alpm_handle_t *handle, alpm_depend_t *dep,
 			if(pkg->name_hash != dep->name_hash && _alpm_depcmp(pkg, dep)
 					&& !alpm_pkg_find(excluding, pkg->name)) {
 				if(alpm_pkg_should_ignore(handle, pkg)) {
-					int install = 0;
+					alpm_question_install_ignorepkg_t question = {
+						.type = ALPM_QUESTION_INSTALL_IGNOREPKG,
+						.install = 0,
+						.pkg = pkg
+					};
 					if(prompt) {
-						QUESTION(handle, ALPM_QUESTION_INSTALL_IGNOREPKG,
+						QUESTION(handle, &question);
-									pkg, NULL, NULL, &install);
 					} else {
 						_alpm_log(handle, ALPM_LOG_WARNING, _("ignoring package %s-%s\n"),
 								pkg->name, pkg->version);
 					}
-					if(!install) {
+					if(!question.install) {
 						ignored = 1;
 						continue;
 					}
@@ -700,15 +706,19 @@ static alpm_pkg_t *resolvedep(alpm_handle_t *handle, alpm_depend_t *dep,
 	}
 	count = alpm_list_count(providers);
 	if(count >= 1) {
+		alpm_question_select_provider_t question = {
+			.type = ALPM_QUESTION_SELECT_PROVIDER,
 			/* default to first provider if there is no QUESTION callback */
-		int idx = 0;
+			.use_index = 0,
+			.providers = providers,
+			.depend = dep
+		};
 		if(count > 1) {
 			/* if there is more than one provider, we ask the user */
-			QUESTION(handle, ALPM_QUESTION_SELECT_PROVIDER,
+			QUESTION(handle, &question);
-					providers, dep, NULL, &idx);
 		}
-		if(idx >= 0 && idx < count) {
+		if(question.use_index >= 0 && question.use_index < count) {
-			alpm_list_t *nth = alpm_list_nth(providers, idx);
+			alpm_list_t *nth = alpm_list_nth(providers, question.use_index);
 			alpm_pkg_t *pkg = nth->data;
 			alpm_list_free(providers);
 			return pkg;

lib/libalpm/handle.h

@@ -37,10 +37,10 @@ do { \
 		(h)->eventcb((alpm_event_t *) (e)); \
 	} \
 } while(0)
-#define QUESTION(h, q, d1, d2, d3, r) \
+#define QUESTION(h, q) \
 do { \
 	if((h)->questioncb) { \
-		(h)->questioncb(q, d1, d2, d3, r); \
+		(h)->questioncb((alpm_question_t *) (q)); \
 	} \
 } while(0)
 #define PROGRESS(h, e, p, per, n, r) \

lib/libalpm/signing.c

@@ -417,7 +417,7 @@ gpg_error:
  */
 int _alpm_key_import(alpm_handle_t *handle, const char *fpr)
 {
-	int answer = 0, ret = -1;
+	int ret = -1;
 	alpm_pgpkey_t fetch_key;
 	memset(&fetch_key, 0, sizeof(fetch_key));
 
@@ -425,9 +425,13 @@ int _alpm_key_import(alpm_handle_t *handle, const char *fpr)
 		_alpm_log(handle, ALPM_LOG_DEBUG,
 				"unknown key, found %s on keyserver\n", fetch_key.uid);
 		if(!_alpm_access(handle, handle->gpgdir, "pubring.gpg", W_OK)) {
-			QUESTION(handle, ALPM_QUESTION_IMPORT_KEY,
+			alpm_question_import_key_t question = {
-					&fetch_key, NULL, NULL, &answer);
+				.type = ALPM_QUESTION_IMPORT_KEY,
-			if(answer) {
+				.import = 0,
+				.key = &fetch_key
+			};
+			QUESTION(handle, &question);
+			if(question.import) {
 				if(key_import(handle, &fetch_key) == 0) {
 					ret = 0;
 				} else {

lib/libalpm/sync.c

@@ -145,7 +145,13 @@ static alpm_list_t *check_replacers(alpm_handle_t *handle, alpm_pkg_t *lpkg,
 			}
 		}
 		if(found) {
-			int doreplace = 0;
+			alpm_question_replace_t question = {
+				.type = ALPM_QUESTION_REPLACE_PKG,
+				.replace = 0,
+				.oldpkg = lpkg,
+				.newpkg = spkg,
+				.newdb = sdb
+			};
 			alpm_pkg_t *tpkg;
 			/* check IgnorePkg/IgnoreGroup */
 			if(alpm_pkg_should_ignore(handle, spkg)
@@ -156,9 +162,8 @@ static alpm_list_t *check_replacers(alpm_handle_t *handle, alpm_pkg_t *lpkg,
 				continue;
 			}
 
-			QUESTION(handle, ALPM_QUESTION_REPLACE_PKG, lpkg, spkg,
+			QUESTION(handle, &question);
-					sdb->treename, &doreplace);
+			if(!question.replace) {
-			if(!doreplace) {
 				continue;
 			}
 
@@ -276,11 +281,14 @@ alpm_list_t SYMEXPORT *alpm_find_group_pkgs(alpm_list_t *dbs,
 				continue;
 			}
 			if(alpm_pkg_should_ignore(db->handle, pkg)) {
+				alpm_question_install_ignorepkg_t question = {
+					.type = ALPM_QUESTION_INSTALL_IGNOREPKG,
+					.install = 0,
+					.pkg = pkg
+				};
 				ignorelist = alpm_list_add(ignorelist, pkg);
-				int install = 0;
+				QUESTION(db->handle, &question);
-				QUESTION(db->handle, ALPM_QUESTION_INSTALL_IGNOREPKG, pkg,
+				if(!question.install)
-						NULL, NULL, &install);
-				if(!install)
 					continue;
 			}
 			if(!alpm_pkg_find(pkgs, pkg->name)) {
@@ -443,10 +451,13 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data)
 		/* If there were unresolvable top-level packages, prompt the user to
 		   see if they'd like to ignore them rather than failing the sync */
 		if(unresolvable != NULL) {
-			int remove_unresolvable = 0;
+			alpm_question_remove_pkgs_t question = {
-			QUESTION(handle, ALPM_QUESTION_REMOVE_PKGS, unresolvable,
+				.type = ALPM_QUESTION_REMOVE_PKGS,
-					NULL, NULL, &remove_unresolvable);
+				.skip = 0,
-			if(remove_unresolvable) {
+				.packages = unresolvable
+			};
+			QUESTION(handle, &question);
+			if(question.skip) {
 				/* User wants to remove the unresolvable packages from the
 				   transaction. The packages will be removed from the actual
 				   transaction when the transaction packages are replaced with a
@@ -560,8 +571,12 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data)
 		deps = _alpm_outerconflicts(handle->db_local, trans->add);
 
 		for(i = deps; i; i = i->next) {
+			alpm_question_conflict_t question = {
+				.type = ALPM_QUESTION_CONFLICT_PKG,
+				.remove = 0,
+				.conflict = i->data
+			};
 			alpm_conflict_t *conflict = i->data;
-			int doremove = 0;
 			int found = 0;
 
 			/* if conflict->package2 (the local package) is not elected for removal,
@@ -582,9 +597,8 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data)
 			_alpm_log(handle, ALPM_LOG_DEBUG, "package '%s' conflicts with '%s'\n",
 					conflict->package1, conflict->package2);
 
-			QUESTION(handle, ALPM_QUESTION_CONFLICT_PKG, conflict->package1,
+			QUESTION(handle, &question);
-							conflict->package2, conflict->reason->name, &doremove);
+			if(question.remove) {
-			if(doremove) {
 				/* append to the removes list */
 				alpm_pkg_t *sync = alpm_pkg_find(trans->add, conflict->package1);
 				alpm_pkg_t *local = _alpm_db_get_pkgfromcache(handle->db_local, conflict->package2);
@@ -793,13 +807,17 @@ static int apply_deltas(alpm_handle_t *handle)
 static int prompt_to_delete(alpm_handle_t *handle, const char *filepath,
 		alpm_errno_t reason)
 {
-	int doremove = 0;
+	alpm_question_corrupted_t question = {
-	QUESTION(handle, ALPM_QUESTION_CORRUPTED_PKG, (char *)filepath,
+		.type = ALPM_QUESTION_CORRUPTED_PKG,
-			&reason, NULL, &doremove);
+		.remove = 0,
-	if(doremove) {
+		.filepath = filepath,
+		.reason = reason
+	};
+	QUESTION(handle, &question);
+	if(question.remove) {
 		unlink(filepath);
 	}
-	return doremove;
+	return question.remove;
 }
 
 static int validate_deltas(alpm_handle_t *handle, alpm_list_t *deltas)

src/pacman/callback.c

@@ -361,55 +361,62 @@ void cb_event(alpm_event_t *event)
 }
 
 /* callback to handle questions from libalpm transactions (yes/no) */
-/* TODO this is one of the worst ever functions written. void *data ? wtf */
+void cb_question(alpm_question_t *question)
-void cb_question(alpm_question_t event, void *data1, void *data2,
-                   void *data3, int *response)
 {
 	if(config->print) {
-		if(event == ALPM_QUESTION_INSTALL_IGNOREPKG) {
+		if(question->type == ALPM_QUESTION_INSTALL_IGNOREPKG) {
-			*response = 1;
+			question->any.answer = 1;
 		} else {
-			*response = 0;
+			question->any.answer = 0;
 		}
 		return;
 	}
-	switch(event) {
+	switch(question->type) {
 		case ALPM_QUESTION_INSTALL_IGNOREPKG:
+			{
+				alpm_question_install_ignorepkg_t *q = &question->install_ignorepkg;
 				if(!config->op_s_downloadonly) {
-				*response = yesno(_("%s is in IgnorePkg/IgnoreGroup. Install anyway?"),
+					q->install = yesno(_("%s is in IgnorePkg/IgnoreGroup. Install anyway?"),
-								alpm_pkg_get_name(data1));
+							alpm_pkg_get_name(q->pkg));
 				} else {
-				*response = 1;
+					q->install = 1;
+				}
 			}
 			break;
 		case ALPM_QUESTION_REPLACE_PKG:
-			*response = yesno(_("Replace %s with %s/%s?"),
+			{
-					alpm_pkg_get_name(data1),
+				alpm_question_replace_t *q = &question->replace;
-					(char *)data3,
+				q->replace = yesno(_("Replace %s with %s/%s?"),
-					alpm_pkg_get_name(data2));
+						alpm_pkg_get_name(q->oldpkg),
+						alpm_db_get_name(q->newdb),
+						alpm_pkg_get_name(q->newpkg));
+			}
 			break;
 		case ALPM_QUESTION_CONFLICT_PKG:
-			/* data parameters: target package, local package, conflict (strings) */
+			{
+				alpm_question_conflict_t *q = &question->conflict;
 				/* print conflict only if it contains new information */
-			if(strcmp(data1, data3) == 0 || strcmp(data2, data3) == 0) {
+				if(strcmp(q->conflict->package1, q->conflict->reason->name) == 0
-				*response = noyes(_("%s and %s are in conflict. Remove %s?"),
+						|| strcmp(q->conflict->package2, q->conflict->reason->name) == 0) {
-						(char *)data1,
+					q->remove = noyes(_("%s and %s are in conflict. Remove %s?"),
-						(char *)data2,
+							q->conflict->package1,
-						(char *)data2);
+							q->conflict->package2,
+							q->conflict->package2);
 				} else {
-				*response = noyes(_("%s and %s are in conflict (%s). Remove %s?"),
+					q->remove = noyes(_("%s and %s are in conflict (%s). Remove %s?"),
-						(char *)data1,
+							q->conflict->package1,
-						(char *)data2,
+							q->conflict->package2,
-						(char *)data3,
+							q->conflict->reason->name,
-						(char *)data2);
+							q->conflict->package2);
+				}
 			}
 			break;
 		case ALPM_QUESTION_REMOVE_PKGS:
 			{
-				alpm_list_t *unresolved = data1;
+				alpm_question_remove_pkgs_t *q = &question->remove_pkgs;
 				alpm_list_t *namelist = NULL, *i;
 				size_t count = 0;
-				for(i = unresolved; i; i = i->next) {
+				for(i = q->packages; i; i = i->next) {
 					namelist = alpm_list_add(namelist,
 							(char *)alpm_pkg_get_name(i->data));
 					count++;
@@ -420,7 +427,7 @@ void cb_question(alpm_question_t event, void *data1, void *data2,
 							count));
 				list_display("     ", namelist, getcols(fileno(stdout)));
 				printf("\n");
-				*response = noyes(_n(
+				q->skip = noyes(_n(
 							"Do you want to skip the above package for this upgrade?",
 							"Do you want to skip the above packages for this upgrade?",
 							count));
@@ -429,43 +436,46 @@ void cb_question(alpm_question_t event, void *data1, void *data2,
 			break;
 		case ALPM_QUESTION_SELECT_PROVIDER:
 			{
-				alpm_list_t *providers = data1;
+				alpm_question_select_provider_t *q = &question->select_provider;
-				size_t count = alpm_list_count(providers);
+				size_t count = alpm_list_count(q->providers);
-				char *depstring = alpm_dep_compute_string((alpm_depend_t *)data2);
+				char *depstring = alpm_dep_compute_string(q->depend);
 				colon_printf(_("There are %zd providers available for %s:\n"), count,
 						depstring);
 				free(depstring);
-				select_display(providers);
+				select_display(q->providers);
-				*response = select_question(count);
+				q->use_index = select_question(count);
 			}
 			break;
 		case ALPM_QUESTION_CORRUPTED_PKG:
-			*response = yesno(_("File %s is corrupted (%s).\n"
+			{
+				alpm_question_corrupted_t *q = &question->corrupted;
+				q->remove = yesno(_("File %s is corrupted (%s).\n"
 							"Do you want to delete it?"),
-					(char *)data1,
+						q->filepath,
-					alpm_strerror(*(alpm_errno_t *)data2));
+						alpm_strerror(q->reason));
+			}
 			break;
 		case ALPM_QUESTION_IMPORT_KEY:
 			{
-				alpm_pgpkey_t *key = data1;
+				alpm_question_import_key_t *q = &question->import_key;
 				char created[12];
-				time_t time = (time_t)key->created;
+				time_t time = (time_t)q->key->created;
 				strftime(created, 12, "%Y-%m-%d", localtime(&time));
 
-				if(key->revoked) {
+				if(q->key->revoked) {
-					*response = yesno(_("Import PGP key %d%c/%s, \"%s\", created: %s (revoked)?"),
+					q->import = yesno(_("Import PGP key %d%c/%s, \"%s\", created: %s (revoked)?"),
-							key->length, key->pubkey_algo, key->fingerprint, key->uid, created);
+							q->key->length, q->key->pubkey_algo, q->key->fingerprint, q->key->uid, created);
 				} else {
-					*response = yesno(_("Import PGP key %d%c/%s, \"%s\", created: %s?"),
+					q->import = yesno(_("Import PGP key %d%c/%s, \"%s\", created: %s?"),
-							key->length, key->pubkey_algo, key->fingerprint, key->uid, created);
+							q->key->length, q->key->pubkey_algo, q->key->fingerprint, q->key->uid, created);
 				}
 			}
 			break;
 	}
 	if(config->noask) {
-		if(config->ask & event) {
+		if(config->ask & question->type) {
 			/* inverse the default answer */
-			*response = !*response;
+			question->any.answer = !question->any.answer;
 		}
 	}
 }

src/pacman/callback.h

@@ -28,8 +28,7 @@
 void cb_event(alpm_event_t *event);
 
 /* callback to handle questions from libalpm (yes/no) */
-void cb_question(alpm_question_t event, void *data1, void *data2,
+void cb_question(alpm_question_t* question);
-                   void *data3, int *response);
 
 /* callback to handle display of progress */
 void cb_progress(alpm_progress_t event, const char *pkgname, int percent,