[pacman-dev] [PATCH v3 2/2] Update the question callback
Olivier Brunel
jjk at jjacky.com
Sat May 31 13:28:03 EDT 2014
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 at jjacky.com>
---
This uses for alpm_quesion_t an union with all question-specific struct, I'm
thinking this should resolve the issue reported with clang.
I don't actually have clang here though, Andrew could you try it and let me
know whether it solves the issue or not, please?
If it does, I'll do a patch with similar changes for events.
-j
lib/libalpm/alpm.h | 104 +++++++++++++++++++++++++++++++++++++++++++++--
lib/libalpm/deps.c | 38 ++++++++++-------
lib/libalpm/handle.h | 4 +-
lib/libalpm/signing.c | 12 ++++--
lib/libalpm/sync.c | 60 +++++++++++++++++----------
src/pacman/callback.c | 110 +++++++++++++++++++++++++++-----------------------
src/pacman/callback.h | 3 +-
7 files changed, 234 insertions(+), 97 deletions(-)
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 66bb5f9..c81b1f2 100644
--- a/lib/libalpm/alpm.h
+++ b/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 {
- ALPM_QUESTION_INSTALL_IGNOREPKG = 1,
+typedef enum _alpm_question_type_t {
+ 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 {
diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c
index b3de1b0..1cbbc5f 100644
--- a/lib/libalpm/deps.c
+++ b/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,
- NULL, NULL, &install);
+ QUESTION(handle, &question);
} 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,
- pkg, NULL, NULL, &install);
+ QUESTION(handle, &question);
} 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) {
- /* default to first provider if there is no QUESTION callback */
- int idx = 0;
+ alpm_question_select_provider_t question = {
+ .type = ALPM_QUESTION_SELECT_PROVIDER,
+ /* default to first provider if there is no QUESTION callback */
+ .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,
- providers, dep, NULL, &idx);
+ QUESTION(handle, &question);
}
- if(idx >= 0 && idx < count) {
- alpm_list_t *nth = alpm_list_nth(providers, idx);
+ if(question.use_index >= 0 && question.use_index < count) {
+ alpm_list_t *nth = alpm_list_nth(providers, question.use_index);
alpm_pkg_t *pkg = nth->data;
alpm_list_free(providers);
return pkg;
diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h
index 27241ea..85c64f6 100644
--- a/lib/libalpm/handle.h
+++ b/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) \
diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c
index caacd24..cf3010f 100644
--- a/lib/libalpm/signing.c
+++ b/lib/libalpm/signing.c
@@ -413,7 +413,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));
@@ -421,9 +421,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,
- &fetch_key, NULL, NULL, &answer);
- if(answer) {
+ alpm_question_import_key_t question = {
+ .type = ALPM_QUESTION_IMPORT_KEY,
+ .import = 0,
+ .key = &fetch_key
+ };
+ QUESTION(handle, &question);
+ if(question.import) {
if(key_import(handle, &fetch_key) == 0) {
ret = 0;
} else {
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index a025b68..38e8f9e 100644
--- a/lib/libalpm/sync.c
+++ b/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,
- sdb->treename, &doreplace);
- if(!doreplace) {
+ QUESTION(handle, &question);
+ if(!question.replace) {
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, ALPM_QUESTION_INSTALL_IGNOREPKG, pkg,
- NULL, NULL, &install);
- if(!install)
+ QUESTION(db->handle, &question);
+ if(!question.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;
- QUESTION(handle, ALPM_QUESTION_REMOVE_PKGS, unresolvable,
- NULL, NULL, &remove_unresolvable);
- if(remove_unresolvable) {
+ alpm_question_remove_pkgs_t question = {
+ .type = ALPM_QUESTION_REMOVE_PKGS,
+ .skip = 0,
+ .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,
- conflict->package2, conflict->reason->name, &doremove);
- if(doremove) {
+ QUESTION(handle, &question);
+ if(question.remove) {
/* 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;
- QUESTION(handle, ALPM_QUESTION_CORRUPTED_PKG, (char *)filepath,
- &reason, NULL, &doremove);
- if(doremove) {
+ alpm_question_corrupted_t question = {
+ .type = ALPM_QUESTION_CORRUPTED_PKG,
+ .remove = 0,
+ .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)
diff --git a/src/pacman/callback.c b/src/pacman/callback.c
index 340a3a1..cb4aaf0 100644
--- a/src/pacman/callback.c
+++ b/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 event, void *data1, void *data2,
- void *data3, int *response)
+void cb_question(alpm_question_t *question)
{
if(config->print) {
- if(event == ALPM_QUESTION_INSTALL_IGNOREPKG) {
- *response = 1;
+ if(question->type == ALPM_QUESTION_INSTALL_IGNOREPKG) {
+ question->any.answer = 1;
} else {
- *response = 0;
+ question->any.answer = 0;
}
return;
}
- switch(event) {
+ switch(question->type) {
case ALPM_QUESTION_INSTALL_IGNOREPKG:
- if(!config->op_s_downloadonly) {
- *response = yesno(_("%s is in IgnorePkg/IgnoreGroup. Install anyway?"),
- alpm_pkg_get_name(data1));
- } else {
- *response = 1;
+ {
+ alpm_question_install_ignorepkg_t *q = (alpm_question_install_ignorepkg_t *) question;
+ if(!config->op_s_downloadonly) {
+ q->install = yesno(_("%s is in IgnorePkg/IgnoreGroup. Install anyway?"),
+ alpm_pkg_get_name(q->pkg));
+ } else {
+ q->install = 1;
+ }
}
break;
case ALPM_QUESTION_REPLACE_PKG:
- *response = yesno(_("Replace %s with %s/%s?"),
- alpm_pkg_get_name(data1),
- (char *)data3,
- alpm_pkg_get_name(data2));
+ {
+ alpm_question_replace_t *q = (alpm_question_replace_t *) question;
+ q->replace = yesno(_("Replace %s with %s/%s?"),
+ 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) */
- /* print conflict only if it contains new information */
- if(strcmp(data1, data3) == 0 || strcmp(data2, data3) == 0) {
- *response = noyes(_("%s and %s are in conflict. Remove %s?"),
- (char *)data1,
- (char *)data2,
- (char *)data2);
- } else {
- *response = noyes(_("%s and %s are in conflict (%s). Remove %s?"),
- (char *)data1,
- (char *)data2,
- (char *)data3,
- (char *)data2);
+ {
+ alpm_question_conflict_t *q = (alpm_question_conflict_t *) question;
+ /* print conflict only if it contains new information */
+ if(strcmp(q->conflict->package1, q->conflict->reason->name) == 0
+ || strcmp(q->conflict->package2, q->conflict->reason->name) == 0) {
+ q->remove = noyes(_("%s and %s are in conflict. Remove %s?"),
+ q->conflict->package1,
+ q->conflict->package2,
+ q->conflict->package2);
+ } else {
+ q->remove = noyes(_("%s and %s are in conflict (%s). Remove %s?"),
+ q->conflict->package1,
+ q->conflict->package2,
+ q->conflict->reason->name,
+ q->conflict->package2);
+ }
}
break;
case ALPM_QUESTION_REMOVE_PKGS:
{
- alpm_list_t *unresolved = data1;
+ alpm_question_remove_pkgs_t *q = (alpm_question_remove_pkgs_t *) question;
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;
- size_t count = alpm_list_count(providers);
- char *depstring = alpm_dep_compute_string((alpm_depend_t *)data2);
+ alpm_question_select_provider_t *q = (alpm_question_select_provider_t *) question;
+ size_t count = alpm_list_count(q->providers);
+ 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);
- *response = select_question(count);
+ select_display(q->providers);
+ q->use_index = select_question(count);
}
break;
case ALPM_QUESTION_CORRUPTED_PKG:
- *response = yesno(_("File %s is corrupted (%s).\n"
- "Do you want to delete it?"),
- (char *)data1,
- alpm_strerror(*(alpm_errno_t *)data2));
+ {
+ alpm_question_corrupted_t *q = (alpm_question_corrupted_t *) question;
+ q->remove = yesno(_("File %s is corrupted (%s).\n"
+ "Do you want to delete it?"),
+ q->filepath,
+ alpm_strerror(q->reason));
+ }
break;
case ALPM_QUESTION_IMPORT_KEY:
{
- alpm_pgpkey_t *key = data1;
+ alpm_question_import_key_t *q = (alpm_question_import_key_t *) question;
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) {
- *response = yesno(_("Import PGP key %d%c/%s, \"%s\", created: %s (revoked)?"),
- key->length, key->pubkey_algo, key->fingerprint, key->uid, created);
+ if(q->key->revoked) {
+ q->import = yesno(_("Import PGP key %d%c/%s, \"%s\", created: %s (revoked)?"),
+ 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?"),
- key->length, key->pubkey_algo, key->fingerprint, key->uid, created);
+ q->import = yesno(_("Import PGP key %d%c/%s, \"%s\", created: %s?"),
+ 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;
}
}
}
diff --git a/src/pacman/callback.h b/src/pacman/callback.h
index f16f7fc..e4941fc 100644
--- a/src/pacman/callback.h
+++ b/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 *data3, int *response);
+void cb_question(alpm_question_t* question);
/* callback to handle display of progress */
void cb_progress(alpm_progress_t event, const char *pkgname, int percent,
--
1.9.3
More information about the pacman-dev
mailing list