[pacman-dev] [PATCH] alpm: Fix possible alignment issues w/ events
Rikard Falkeborn
rikard.falkeborn at gmail.com
Sat Jan 2 19:50:03 UTC 2016
2016-01-02 19:21 GMT+01:00 Olivier Brunel <jjk at jjacky.com>:
> As reported by Rikard Falkeborn[1] using event-specific struct and then
> typecasting to the generic alpm_event_t could possibly lead to alignment
> issue,
> so we now always use alpm_event_t instead.
>
> [1]
> https://lists.archlinux.org/pipermail/pacman-dev/2015-December/020709.html
>
> Signed-off-by: Olivier Brunel <jjk at jjacky.com>
> ---
> lib/libalpm/add.c | 36 ++++++++++++++++++------------------
> lib/libalpm/be_sync.c | 4 ++--
> lib/libalpm/handle.h | 2 +-
> lib/libalpm/hook.c | 14 +++++++-------
> lib/libalpm/remove.c | 20 ++++++++++----------
> lib/libalpm/sync.c | 8 ++++----
> lib/libalpm/trans.c | 2 +-
> lib/libalpm/util.c | 4 ++--
> 8 files changed, 45 insertions(+), 45 deletions(-)
>
> diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
> index 63eda49..4b76bc7 100644
> --- a/lib/libalpm/add.c
> +++ b/lib/libalpm/add.c
> @@ -315,12 +315,12 @@ static int extract_single_file(alpm_handle_t
> *handle, struct archive *archive,
> }
>
> if(notouch) {
> - alpm_event_pacnew_created_t event = {
> + alpm_event_t event = {
> .type = ALPM_EVENT_PACNEW_CREATED,
> - .from_noupgrade = 1,
> - .oldpkg = oldpkg,
> - .newpkg = newpkg,
> - .file = filename
> + .pacnew_created.from_noupgrade = 1,
> + .pacnew_created.oldpkg = oldpkg,
> + .pacnew_created.newpkg = newpkg,
> + .pacnew_created.file = filename
> };
> /* "remove" the .pacnew suffix */
> filename[filename_len] = '\0';
> @@ -366,12 +366,12 @@ static int extract_single_file(alpm_handle_t
> *handle, struct archive *archive,
> } else {
> /* none of the three files matched another, leave
> the unpacked
> * file alongside the local file */
> - alpm_event_pacnew_created_t event = {
> + alpm_event_t event = {
> .type = ALPM_EVENT_PACNEW_CREATED,
> - .from_noupgrade = 0,
> - .oldpkg = oldpkg,
> - .newpkg = newpkg,
> - .file = origfile
> + .pacnew_created.from_noupgrade = 0,
> + .pacnew_created.oldpkg = oldpkg,
> + .pacnew_created.newpkg = newpkg,
> + .pacnew_created.file = origfile
> };
> _alpm_log(handle, ALPM_LOG_DEBUG,
> "action: keeping current file and
> installing"
> @@ -398,7 +398,7 @@ static int commit_single_pkg(alpm_handle_t *handle,
> alpm_pkg_t *newpkg,
> alpm_db_t *db = handle->db_local;
> alpm_trans_t *trans = handle->trans;
> alpm_progress_t progress = ALPM_PROGRESS_ADD_START;
> - alpm_event_package_operation_t event;
> + alpm_event_t event;
> const char *log_msg = "adding";
> const char *pkgfile;
>
> @@ -411,15 +411,15 @@ static int commit_single_pkg(alpm_handle_t *handle,
> alpm_pkg_t *newpkg,
> if(cmp < 0) {
> log_msg = "downgrading";
> progress = ALPM_PROGRESS_DOWNGRADE_START;
> - event.operation = ALPM_PACKAGE_DOWNGRADE;
> + event.package_operation.operation =
> ALPM_PACKAGE_DOWNGRADE;
> } else if(cmp == 0) {
> log_msg = "reinstalling";
> progress = ALPM_PROGRESS_REINSTALL_START;
> - event.operation = ALPM_PACKAGE_REINSTALL;
> + event.package_operation.operation =
> ALPM_PACKAGE_REINSTALL;
> } else {
> log_msg = "upgrading";
> progress = ALPM_PROGRESS_UPGRADE_START;
> - event.operation = ALPM_PACKAGE_UPGRADE;
> + event.package_operation.operation =
> ALPM_PACKAGE_UPGRADE;
> }
> is_upgrade = 1;
>
> @@ -432,12 +432,12 @@ static int commit_single_pkg(alpm_handle_t *handle,
> alpm_pkg_t *newpkg,
> /* copy over the install reason */
> newpkg->reason = alpm_pkg_get_reason(local);
> } else {
> - event.operation = ALPM_PACKAGE_INSTALL;
> + event.package_operation.operation = ALPM_PACKAGE_INSTALL;
> }
>
> event.type = ALPM_EVENT_PACKAGE_OPERATION_START;
> - event.oldpkg = oldpkg;
> - event.newpkg = newpkg;
> + event.package_operation.oldpkg = oldpkg;
> + event.package_operation.newpkg = newpkg;
> EVENT(handle, &event);
>
> pkgfile = newpkg->origin_data.file;
> @@ -589,7 +589,7 @@ static int commit_single_pkg(alpm_handle_t *handle,
> alpm_pkg_t *newpkg,
>
> PROGRESS(handle, progress, newpkg->name, 100, pkg_count,
> pkg_current);
>
> - switch(event.operation) {
> + switch(event.package_operation.operation) {
> case ALPM_PACKAGE_INSTALL:
> alpm_logaction(handle, ALPM_CALLER_PREFIX,
> "installed %s (%s)\n",
> newpkg->name, newpkg->version);
> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index 200e381..2ad01f5 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
> @@ -90,9 +90,9 @@ static int sync_db_validate(alpm_db_t *db)
>
> /* we can skip any validation if the database doesn't exist */
> if(_alpm_access(db->handle, NULL, dbpath, R_OK) != 0 && errno ==
> ENOENT) {
> - alpm_event_database_missing_t event = {
> + alpm_event_t event = {
> .type = ALPM_EVENT_DATABASE_MISSING,
> - .dbname = db->treename
> + .database_missing.dbname = db->treename
> };
> db->status &= ~DB_STATUS_EXISTS;
> db->status |= DB_STATUS_MISSING;
> diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h
> index e252fbf..2682fd4 100644
> --- a/lib/libalpm/handle.h
> +++ b/lib/libalpm/handle.h
> @@ -34,7 +34,7 @@
> #define EVENT(h, e) \
> do { \
> if((h)->eventcb) { \
> - (h)->eventcb((alpm_event_t *) (e)); \
> + (h)->eventcb(e); \
> } \
> } while(0)
> #define QUESTION(h, q) \
> diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c
> index b5ed17d..e9b50e8 100644
> --- a/lib/libalpm/hook.c
> +++ b/lib/libalpm/hook.c
> @@ -614,8 +614,8 @@ static int _alpm_hook_run_hook(alpm_handle_t *handle,
> struct _alpm_hook_t *hook)
>
> int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when)
> {
> - alpm_event_hook_t event = { .when = when };
> - alpm_event_hook_run_t hook_event;
> + alpm_event_t event = { .hook.when = when };
> + alpm_event_t hook_event;
> alpm_list_t *i, *hooks = NULL, *hooks_triggered = NULL;
> const char *suffix = ".hook";
> size_t suflen = strlen(suffix), triggered = 0;
> @@ -731,16 +731,16 @@ int _alpm_hook_run(alpm_handle_t *handle,
> alpm_hook_when_t when)
> event.type = ALPM_EVENT_HOOK_START;
> EVENT(handle, &event);
>
> - hook_event.position = 1;
> - hook_event.total = triggered;
> + hook_event.hook_run.position = 1;
> + hook_event.hook_run.total = triggered;
>
> - for(i = hooks_triggered; i; i = i->next,
> hook_event.position++) {
> + for(i = hooks_triggered; i; i = i->next,
> hook_event.hook_run.position++) {
> struct _alpm_hook_t *hook = i->data;
> _alpm_log(handle, ALPM_LOG_DEBUG, "running hook
> %s\n", hook->name);
>
> hook_event.type = ALPM_EVENT_HOOK_RUN_START;
> - hook_event.name = hook->name;
> - hook_event.desc = hook->desc;
> + hook_event.hook_run.name = hook->name;
> + hook_event.hook_run.desc = hook->desc;
> EVENT(handle, &hook_event);
>
> if(_alpm_hook_run_hook(handle, hook) != 0 &&
> hook->abort_on_fail) {
> diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c
> index 2fb7993..c7473e4 100644
> --- a/lib/libalpm/remove.c
> +++ b/lib/libalpm/remove.c
> @@ -181,10 +181,10 @@ static void
> remove_notify_needed_optdepends(alpm_handle_t *handle, alpm_list_t *
> for(j = optdeps; j; j = alpm_list_next(j)) {
> alpm_depend_t *optdep = j->data;
> if(alpm_pkg_find(lp, optdep->name)) {
> - alpm_event_optdep_removal_t event
> = {
> + alpm_event_t event = {
> .type =
> ALPM_EVENT_OPTDEP_REMOVAL,
> - .pkg = pkg,
> - .optdep = optdep
> + .optdep_removal.pkg = pkg,
> + .optdep_removal.optdep =
> optdep
> };
> EVENT(handle, &event);
> }
> @@ -506,10 +506,10 @@ static int unlink_file(alpm_handle_t *handle,
> alpm_pkg_t *oldpkg,
> int cmp = filehash ? strcmp(filehash,
> backup->hash) : 0;
> FREE(filehash);
> if(cmp != 0) {
> - alpm_event_pacsave_created_t event
> = {
> + alpm_event_t event = {
> .type =
> ALPM_EVENT_PACSAVE_CREATED,
> - .oldpkg = oldpkg,
> - .file = file
> + .pacsave_created.oldpkg =
> oldpkg,
> + .pacsave_created.file =
> file
> };
> char *newpath;
> size_t len = strlen(file) + 8 + 1;
> @@ -658,11 +658,11 @@ int _alpm_remove_single_package(alpm_handle_t
> *handle,
> {
> const char *pkgname = oldpkg->name;
> const char *pkgver = oldpkg->version;
> - alpm_event_package_operation_t event = {
> + alpm_event_t event = {
> .type = ALPM_EVENT_PACKAGE_OPERATION_START,
> - .operation = ALPM_PACKAGE_REMOVE,
> - .oldpkg = oldpkg,
> - .newpkg = NULL
> + .package_operation.operation = ALPM_PACKAGE_REMOVE,
> + .package_operation.oldpkg = oldpkg,
> + .package_operation.newpkg = NULL
> };
>
> if(newpkg) {
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index c5607bc..27528d5 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -704,7 +704,7 @@ static int apply_deltas(alpm_handle_t *handle)
> int ret = 0;
> const char *cachedir = _alpm_filecache_setup(handle);
> alpm_trans_t *trans = handle->trans;
> - alpm_event_delta_patch_t event;
> + alpm_event_t event;
>
> for(i = trans->add; i; i = i->next) {
> alpm_pkg_t *spkg = i->data;
> @@ -754,7 +754,7 @@ static int apply_deltas(alpm_handle_t *handle)
> _alpm_log(handle, ALPM_LOG_DEBUG, "command: %s\n",
> command);
>
> event.type = ALPM_EVENT_DELTA_PATCH_START;
> - event.delta = d;
> + event.delta_patch.delta = d;
> EVENT(handle, &event);
>
> int retval = system(command);
> @@ -917,9 +917,9 @@ static int find_dl_candidates(alpm_db_t *repo,
> alpm_list_t **files, alpm_list_t
> static int download_single_file(alpm_handle_t *handle, struct
> dload_payload *payload,
> const char *cachedir)
> {
> - alpm_event_pkgdownload_t event = {
> + alpm_event_t event = {
> .type = ALPM_EVENT_PKGDOWNLOAD_START,
> - .file = payload->remote_name
> + .pkgdownload.file = payload->remote_name
> };
> const alpm_list_t *server;
>
> diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c
> index 5b52049..bfb3115 100644
> --- a/lib/libalpm/trans.c
> +++ b/lib/libalpm/trans.c
> @@ -160,7 +160,7 @@ int SYMEXPORT alpm_trans_prepare(alpm_handle_t
> *handle, alpm_list_t **data)
> int SYMEXPORT alpm_trans_commit(alpm_handle_t *handle, alpm_list_t **data)
> {
> alpm_trans_t *trans;
> - alpm_event_any_t event;
> + alpm_event_t event;
>
> /* Sanity checks */
> CHECK_HANDLE(handle, return -1);
> diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
> index 3824d13..6d14da7 100644
> --- a/lib/libalpm/util.c
> +++ b/lib/libalpm/util.c
> @@ -489,9 +489,9 @@ static int _alpm_chroot_write_to_child(alpm_handle_t
> *handle, int fd,
>
> static void _alpm_chroot_process_output(alpm_handle_t *handle, const char
> *line)
> {
> - alpm_event_scriptlet_info_t event = {
> + alpm_event_t event = {
> .type = ALPM_EVENT_SCRIPTLET_INFO,
> - .line = line
> + .scriptlet_info.line = line
> };
> alpm_logaction(handle, "ALPM-SCRIPTLET", "%s", line);
> EVENT(handle, &event);
> --
> 2.6.4
>
Hello,
Clang is still not happy, same types of warnings in all the files changed
(this output from just running ./configure CC=clang, i.e., no extra
warningflags enabled):
CC libalpm_la-add.lo
add.c:320:5: warning: initializer overrides prior initialization of this
subobject [-Winitializer-overrides]
.pacnew_created.from_noupgrade = 1,
~^~~~~~~~~~~~~~
add.c:319:12: note: previous initialization is here
.type = ALPM_EVENT_PACNEW_CREATED,
^~~~~~~~~~~~~~~~~~~~~~~~~
add.c:371:6: warning: initializer overrides prior initialization of this
subobject [-Winitializer-overrides]
.pacnew_created.from_noupgrade = 0,
~^~~~~~~~~~~~~~
add.c:370:13: note: previous initialization is here
.type = ALPM_EVENT_PACNEW_CREATED,
^~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.
Changing
.type = ALPM_EVENT_PACNEW_CREATED,
to
.pacnew_created.type = ALPM_EVENT_PACNEW_CREATED,
produces no warnings.
/Rikard
More information about the pacman-dev
mailing list