[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