There are some warnings recently introduced when compiling with Clang (with --enable-warningflags) for x86_64. The warnings are related to casting of specific alpm_event-types to alpm_event_t where the required alignment differs between the types. On x86, it shouldn't be a problem, but maybe there are other architectures pacman should work on where this is a real issue? Either way, I think it would be good if pacman built cleanly with Clang. Any suggestions on how to fix this? For reference, the warnings are: hook.c:732:3: warning: cast from 'alpm_event_hook_t *' (aka 'struct _alpm_event_hook_t *') to 'alpm_event_t *' (aka 'union _alpm_event_t *') increases required alignment from 4 to 8 [-Wcast-align] EVENT(handle, &event); ^~~~~~~~~~~~~~~~~~~~~ ./handle.h:37:16: note: expanded from macro 'EVENT' (h)->eventcb((alpm_event_t *) (e)); \ ^~~~~~~~~~~~~~~~~~~~ hook.c:761:3: warning: cast from 'alpm_event_hook_t *' (aka 'struct _alpm_event_hook_t *') to 'alpm_event_t *' (aka 'union _alpm_event_t *') increases required alignment from 4 to 8 [-Wcast-align] EVENT(handle, &event); ^~~~~~~~~~~~~~~~~~~~~ ./handle.h:37:16: note: expanded from macro 'EVENT' (h)->eventcb((alpm_event_t *) (e)); \ ^~~~~~~~~~~~~~~~~~~~ 2 warnings generated. trans.c:202:2: warning: cast from 'alpm_event_any_t *' (aka 'struct _alpm_event_any_t *') to 'alpm_event_t *' (aka 'union _alpm_event_t *') increases required alignment from 4 to 8 [-Wcast-align] EVENT(handle, &event); ^~~~~~~~~~~~~~~~~~~~~ ./handle.h:37:16: note: expanded from macro 'EVENT' (h)->eventcb((alpm_event_t *) (e)); \ ^~~~~~~~~~~~~~~~~~~~ trans.c:226:3: warning: cast from 'alpm_event_any_t *' (aka 'struct _alpm_event_any_t *') to 'alpm_event_t *' (aka 'union _alpm_event_t *') increases required alignment from 4 to 8 [-Wcast-align] EVENT(handle, &event); ^~~~~~~~~~~~~~~~~~~~~ ./handle.h:37:16: note: expanded from macro 'EVENT' (h)->eventcb((alpm_event_t *) (e)); \ ^~~~~~~~~~~~~~~~~~~~
On Thu, 31 Dec 2015 14:32:45 +0100 Rikard Falkeborn <rikard.falkeborn@gmail.com> wrote:
There are some warnings recently introduced when compiling with Clang (with --enable-warningflags) for x86_64. The warnings are related to casting of specific alpm_event-types to alpm_event_t where the required alignment differs between the types. On x86, it shouldn't be a problem, but maybe there are other architectures pacman should work on where this is a real issue? Either way, I think it would be good if pacman built cleanly with Clang. Any suggestions on how to fix this?
I think the only way is not to use a specific event type but the generic one, to ensure the size is correct. I don't have clang so I didn't test this, but I think there are two ways to solve this. Basically the issue is when using code such as: alpm_event_hook_run_t event; event.type = ALPM_EVENT_HOOK_RUN_START; event.name = name; event.desc = desc; EVENT(handle, &event); So we could either use the alpm_event_t and always go into the right union member, e.g: alpm_event_t event; event.hook_run.type = ALPM_EVENT_HOOK_RUN_START; event.hook_run.name = name; event.hook_run.desc = desc; EVENT(handle, &event); Or we could use a pointer, as that may be a little less verbose, e.g: alpm_event_t _event; alpm_event_hook_run_t *event = &_event.hook_run; event->type = ALPM_EVENT_HOOK_RUN_START; event->name = name; event->desc = desc; EVENT(handle, &_event); I'm not sure if using EVENT(handle, event) would work then, even though they're basically the same in the end. Not sure which solution you guys would prefer, if any? I'd be happy to send a patch addressing this if you'd like though. -j
On 01/01/16 22:01, Olivier Brunel wrote:
On Thu, 31 Dec 2015 14:32:45 +0100 Rikard Falkeborn <rikard.falkeborn@gmail.com> wrote:
There are some warnings recently introduced when compiling with Clang (with --enable-warningflags) for x86_64. The warnings are related to casting of specific alpm_event-types to alpm_event_t where the required alignment differs between the types. On x86, it shouldn't be a problem, but maybe there are other architectures pacman should work on where this is a real issue? Either way, I think it would be good if pacman built cleanly with Clang. Any suggestions on how to fix this?
I think the only way is not to use a specific event type but the generic one, to ensure the size is correct. I don't have clang so I didn't test this, but I think there are two ways to solve this.
Basically the issue is when using code such as:
alpm_event_hook_run_t event; event.type = ALPM_EVENT_HOOK_RUN_START; event.name = name; event.desc = desc; EVENT(handle, &event);
So we could either use the alpm_event_t and always go into the right union member, e.g:
alpm_event_t event; event.hook_run.type = ALPM_EVENT_HOOK_RUN_START; event.hook_run.name = name; event.hook_run.desc = desc; EVENT(handle, &event);
I'd prefer this way but... What is different with these events than all the others? Why do the old ones not give warnings? We should be consistent here. I have a feeling we have had this conversation before... Allan
On Fri, 1 Jan 2016 23:03:22 +1000 Allan McRae <allan@archlinux.org> wrote:
On 01/01/16 22:01, Olivier Brunel wrote:
On Thu, 31 Dec 2015 14:32:45 +0100 Rikard Falkeborn <rikard.falkeborn@gmail.com> wrote:
There are some warnings recently introduced when compiling with Clang (with --enable-warningflags) for x86_64. The warnings are related to casting of specific alpm_event-types to alpm_event_t where the required alignment differs between the types. On x86, it shouldn't be a problem, but maybe there are other architectures pacman should work on where this is a real issue? Either way, I think it would be good if pacman built cleanly with Clang. Any suggestions on how to fix this?
I think the only way is not to use a specific event type but the generic one, to ensure the size is correct. I don't have clang so I didn't test this, but I think there are two ways to solve this.
Basically the issue is when using code such as:
alpm_event_hook_run_t event; event.type = ALPM_EVENT_HOOK_RUN_START; event.name = name; event.desc = desc; EVENT(handle, &event);
So we could either use the alpm_event_t and always go into the right union member, e.g:
alpm_event_t event; event.hook_run.type = ALPM_EVENT_HOOK_RUN_START; event.hook_run.name = name; event.hook_run.desc = desc; EVENT(handle, &event);
I'd prefer this way but...
What is different with these events than all the others? Why do the old ones not give warnings? We should be consistent here. I have a feeling we have had this conversation before...
Allan
I picked hook_run as an example, but it might affect older ones as well. The point is that alpm_event_t and alpm_event_hook_run_t differ in size. Now since the former is a union of all the specific events, it can hold them all, but when creating/using only a specific one, it might be of a different size. (Or maybe this is the first one to grow over the generic event?) As for having had this conversation before, I'm not sure, but there probaly have been a similar one in the other way around: how to use the generic event to get specific values, e.g. in pacman callback handler: typecast, of through the union. For the record, though sometimes we just go through the union (e.g. event->scriptlet_info.line) most times (or, whenever more than one or two values are needed, or used more than once) we just use a specific pointer, e.g: alpm_event_hook_run_t *e = &event->hook_run; -j
Den 1 jan 2016 15:06 skrev "Olivier Brunel" <jjk@jjacky.com>:
On Fri, 1 Jan 2016 23:03:22 +1000 Allan McRae <allan@archlinux.org> wrote:
On 01/01/16 22:01, Olivier Brunel wrote:
On Thu, 31 Dec 2015 14:32:45 +0100 Rikard Falkeborn <rikard.falkeborn@gmail.com> wrote:
There are some warnings recently introduced when compiling with Clang (with --enable-warningflags) for x86_64. The warnings are related to casting of specific alpm_event-types to alpm_event_t where the required alignment differs between the types. On x86, it shouldn't be a problem, but maybe there are other architectures pacman should work on where this is a real issue? Either way, I think it would be good if pacman built cleanly with Clang. Any suggestions on how to fix this?
I think the only way is not to use a specific event type but the generic one, to ensure the size is correct. I don't have clang so I didn't test this, but I think there are two ways to solve this.
Basically the issue is when using code such as:
alpm_event_hook_run_t event; event.type = ALPM_EVENT_HOOK_RUN_START; event.name = name; event.desc = desc; EVENT(handle, &event);
So we could either use the alpm_event_t and always go into the right union member, e.g:
alpm_event_t event; event.hook_run.type = ALPM_EVENT_HOOK_RUN_START; event.hook_run.name = name; event.hook_run.desc = desc; EVENT(handle, &event);
I'd prefer this way but...
What is different with these events than all the others? Why do the old ones not give warnings? We should be consistent here. I have a feeling we have had this conversation before...
Allan
I picked hook_run as an example, but it might affect older ones as well. The point is that alpm_event_t and alpm_event_hook_run_t differ in size. Now since the former is a union of all the specific events, it can hold them all, but when creating/using only a specific one, it might be of a different size. (Or maybe this is the first one to grow over the generic event?)
As for having had this conversation before, I'm not sure, but there probaly have been a similar one in the other way around: how to use the generic event to get specific values, e.g. in pacman callback handler: typecast, of through the union.
For the record, though sometimes we just go through the union (e.g. event->scriptlet_info.line) most times (or, whenever more than one or two values are needed, or used more than once) we just use a specific pointer, e.g: alpm_event_hook_run_t *e = &event->hook_run;
-j
The types clang warns about do not contain pointers, I think every other type does, that's why the required alignment differs. Adding a dummy pointer to them makes the warnings go away. The warning has nothing to do with the actual size of the types but where in memory they are stored (is the adress guaranteed to be an even multiple of four or eight). Rikard
On Fri, 1 Jan 2016 15:19:19 +0100 Rikard Falkeborn <rikard.falkeborn@gmail.com> wrote:
Den 1 jan 2016 15:06 skrev "Olivier Brunel" <jjk@jjacky.com>:
On Fri, 1 Jan 2016 23:03:22 +1000 Allan McRae <allan@archlinux.org> wrote:
On 01/01/16 22:01, Olivier Brunel wrote:
On Thu, 31 Dec 2015 14:32:45 +0100 Rikard Falkeborn <rikard.falkeborn@gmail.com> wrote:
There are some warnings recently introduced when compiling with Clang (with --enable-warningflags) for x86_64. The warnings are related to casting of specific alpm_event-types to alpm_event_t where the required alignment differs between the types. On x86, it shouldn't be a problem, but maybe there are other architectures pacman should work on where this is a real issue? Either way, I think it would be good if pacman built cleanly with Clang. Any suggestions on how to fix this?
I think the only way is not to use a specific event type but the generic one, to ensure the size is correct. I don't have clang so I didn't test this, but I think there are two ways to solve this.
Basically the issue is when using code such as:
alpm_event_hook_run_t event; event.type = ALPM_EVENT_HOOK_RUN_START; event.name = name; event.desc = desc; EVENT(handle, &event);
So we could either use the alpm_event_t and always go into the right union member, e.g:
alpm_event_t event; event.hook_run.type = ALPM_EVENT_HOOK_RUN_START; event.hook_run.name = name; event.hook_run.desc = desc; EVENT(handle, &event);
I'd prefer this way but...
What is different with these events than all the others? Why do the old ones not give warnings? We should be consistent here. I have a feeling we have had this conversation before...
Allan
I picked hook_run as an example, but it might affect older ones as well. The point is that alpm_event_t and alpm_event_hook_run_t differ in size. Now since the former is a union of all the specific events, it can hold them all, but when creating/using only a specific one, it might be of a different size. (Or maybe this is the first one to grow over the generic event?)
As for having had this conversation before, I'm not sure, but there probaly have been a similar one in the other way around: how to use the generic event to get specific values, e.g. in pacman callback handler: typecast, of through the union.
For the record, though sometimes we just go through the union (e.g. event->scriptlet_info.line) most times (or, whenever more than one or two values are needed, or used more than once) we just use a specific pointer, e.g: alpm_event_hook_run_t *e = &event->hook_run;
-j
The types clang warns about do not contain pointers, I think every other type does, that's why the required alignment differs. Adding a dummy pointer to them makes the warnings go away.
The warning has nothing to do with the actual size of the types but where in memory they are stored (is the adress guaranteed to be an even multiple of four or eight).
Rikard
Oh, right, my bad; Apologies. Reading the OP again, I can see that alpm_event_hook_run_t wasn't even at fault BTW, alpm_event_hook_t was; I just used hook_run in my code examples. (alpm_event_any_t was also listed, so an old one, but I guess it was never used that way before (we'd just use alpm_event_t), hence the new warning.) My proposed solutions should still apply though, so it's only a matter of picking a style I guess. -j
On 02/01/16 02:46, Olivier Brunel wrote:
On Fri, 1 Jan 2016 15:19:19 +0100 Rikard Falkeborn <rikard.falkeborn@gmail.com> wrote:
Den 1 jan 2016 15:06 skrev "Olivier Brunel" <jjk@jjacky.com>:
On Fri, 1 Jan 2016 23:03:22 +1000 Allan McRae <allan@archlinux.org> wrote:
On 01/01/16 22:01, Olivier Brunel wrote:
On Thu, 31 Dec 2015 14:32:45 +0100 Rikard Falkeborn <rikard.falkeborn@gmail.com> wrote:
There are some warnings recently introduced when compiling with Clang (with --enable-warningflags) for x86_64. The warnings are related to casting of specific alpm_event-types to alpm_event_t where the required alignment differs between the types. On x86, it shouldn't be a problem, but maybe there are other architectures pacman should work on where this is a real issue? Either way, I think it would be good if pacman built cleanly with Clang. Any suggestions on how to fix this?
I think the only way is not to use a specific event type but the generic one, to ensure the size is correct. I don't have clang so I didn't test this, but I think there are two ways to solve this.
Basically the issue is when using code such as:
alpm_event_hook_run_t event; event.type = ALPM_EVENT_HOOK_RUN_START; event.name = name; event.desc = desc; EVENT(handle, &event);
So we could either use the alpm_event_t and always go into the right union member, e.g:
alpm_event_t event; event.hook_run.type = ALPM_EVENT_HOOK_RUN_START; event.hook_run.name = name; event.hook_run.desc = desc; EVENT(handle, &event);
I'd prefer this way but...
What is different with these events than all the others? Why do the old ones not give warnings? We should be consistent here. I have a feeling we have had this conversation before...
Allan
I picked hook_run as an example, but it might affect older ones as well. The point is that alpm_event_t and alpm_event_hook_run_t differ in size. Now since the former is a union of all the specific events, it can hold them all, but when creating/using only a specific one, it might be of a different size. (Or maybe this is the first one to grow over the generic event?)
As for having had this conversation before, I'm not sure, but there probaly have been a similar one in the other way around: how to use the generic event to get specific values, e.g. in pacman callback handler: typecast, of through the union.
For the record, though sometimes we just go through the union (e.g. event->scriptlet_info.line) most times (or, whenever more than one or two values are needed, or used more than once) we just use a specific pointer, e.g: alpm_event_hook_run_t *e = &event->hook_run;
-j
The types clang warns about do not contain pointers, I think every other type does, that's why the required alignment differs. Adding a dummy pointer to them makes the warnings go away.
The warning has nothing to do with the actual size of the types but where in memory they are stored (is the adress guaranteed to be an even multiple of four or eight).
Rikard
Oh, right, my bad; Apologies.
Reading the OP again, I can see that alpm_event_hook_run_t wasn't even at fault BTW, alpm_event_hook_t was; I just used hook_run in my code examples. (alpm_event_any_t was also listed, so an old one, but I guess it was never used that way before (we'd just use alpm_event_t), hence the new warning.)
My proposed solutions should still apply though, so it's only a matter of picking a style I guess.
Use this style: alpm_event_t event; event.hook_run.type = ALPM_EVENT_HOOK_RUN_START; ... EVENT(handle, &event) Allan
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@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
2016-01-02 19:21 GMT+01:00 Olivier Brunel <jjk@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@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
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@jjacky.com> --- Now always using the union member, to avoid initializer warnings (thanks Rikard) and better consistency. Hopefully this time it's all good. lib/libalpm/add.c | 44 ++++++++++++++++++++++---------------------- lib/libalpm/be_sync.c | 6 +++--- lib/libalpm/handle.h | 2 +- lib/libalpm/hook.c | 22 +++++++++++----------- lib/libalpm/remove.c | 28 ++++++++++++++-------------- lib/libalpm/sync.c | 14 +++++++------- lib/libalpm/trans.c | 2 +- lib/libalpm/util.c | 6 +++--- 8 files changed, 62 insertions(+), 62 deletions(-) diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index 63eda49..6df7d7a 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 = { - .type = ALPM_EVENT_PACNEW_CREATED, - .from_noupgrade = 1, - .oldpkg = oldpkg, - .newpkg = newpkg, - .file = filename + alpm_event_t event = { + .pacnew_created.type = ALPM_EVENT_PACNEW_CREATED, + .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 = { - .type = ALPM_EVENT_PACNEW_CREATED, - .from_noupgrade = 0, - .oldpkg = oldpkg, - .newpkg = newpkg, - .file = origfile + alpm_event_t event = { + .pacnew_created.type = ALPM_EVENT_PACNEW_CREATED, + .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.type = ALPM_EVENT_PACKAGE_OPERATION_START; + 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); @@ -622,7 +622,7 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, free(scriptlet); } - event.type = ALPM_EVENT_PACKAGE_OPERATION_DONE; + event.package_operation.type = ALPM_EVENT_PACKAGE_OPERATION_DONE; EVENT(handle, &event); cleanup: diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index 200e381..313841a 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 = { - .type = ALPM_EVENT_DATABASE_MISSING, - .dbname = db->treename + alpm_event_t event = { + .database_missing.type = ALPM_EVENT_DATABASE_MISSING, + .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..01668d4 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; @@ -728,26 +728,26 @@ int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when) } if(hooks_triggered != NULL) { - event.type = ALPM_EVENT_HOOK_START; + event.hook.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.type = ALPM_EVENT_HOOK_RUN_START; + 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) { ret = -1; } - hook_event.type = ALPM_EVENT_HOOK_RUN_DONE; + hook_event.hook_run.type = ALPM_EVENT_HOOK_RUN_DONE; EVENT(handle, &hook_event); if(ret != 0 && when == ALPM_HOOK_PRE_TRANSACTION) { @@ -757,7 +757,7 @@ int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when) alpm_list_free(hooks_triggered); - event.type = ALPM_EVENT_HOOK_DONE; + event.hook.type = ALPM_EVENT_HOOK_DONE; EVENT(handle, &event); } diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 2fb7993..ad24d6a 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 = { - .type = ALPM_EVENT_OPTDEP_REMOVAL, - .pkg = pkg, - .optdep = optdep + alpm_event_t event = { + .optdep_removal.type = ALPM_EVENT_OPTDEP_REMOVAL, + .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 = { - .type = ALPM_EVENT_PACSAVE_CREATED, - .oldpkg = oldpkg, - .file = file + alpm_event_t event = { + .pacsave_created.type = ALPM_EVENT_PACSAVE_CREATED, + .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 = { - .type = ALPM_EVENT_PACKAGE_OPERATION_START, - .operation = ALPM_PACKAGE_REMOVE, - .oldpkg = oldpkg, - .newpkg = NULL + alpm_event_t event = { + .package_operation.type = ALPM_EVENT_PACKAGE_OPERATION_START, + .package_operation.operation = ALPM_PACKAGE_REMOVE, + .package_operation.oldpkg = oldpkg, + .package_operation.newpkg = NULL }; if(newpkg) { @@ -703,7 +703,7 @@ int _alpm_remove_single_package(alpm_handle_t *handle, } if(!newpkg) { - event.type = ALPM_EVENT_PACKAGE_OPERATION_DONE; + event.package_operation.type = ALPM_EVENT_PACKAGE_OPERATION_DONE; EVENT(handle, &event); } diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index c5607bc..a7582b4 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 = { - .type = ALPM_EVENT_PKGDOWNLOAD_START, - .file = payload->remote_name + alpm_event_t event = { + .pkgdownload.type = ALPM_EVENT_PKGDOWNLOAD_START, + .pkgdownload.file = payload->remote_name }; const alpm_list_t *server; @@ -937,7 +937,7 @@ static int download_single_file(alpm_handle_t *handle, struct dload_payload *pay snprintf(payload->fileurl, len, "%s/%s", server_url, payload->remote_name); if(_alpm_download(payload, cachedir, NULL, NULL) != -1) { - event.type = ALPM_EVENT_PKGDOWNLOAD_DONE; + event.pkgdownload.type = ALPM_EVENT_PKGDOWNLOAD_DONE; EVENT(handle, &event); return 0; } @@ -946,7 +946,7 @@ static int download_single_file(alpm_handle_t *handle, struct dload_payload *pay payload->unlink_on_fail = 0; } - event.type = ALPM_EVENT_PKGDOWNLOAD_FAILED; + event.pkgdownload.type = ALPM_EVENT_PKGDOWNLOAD_FAILED; EVENT(handle, &event); return -1; } 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..0967951 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 = { - .type = ALPM_EVENT_SCRIPTLET_INFO, - .line = line + alpm_event_t event = { + .scriptlet_info.type = ALPM_EVENT_SCRIPTLET_INFO, + .scriptlet_info.line = line }; alpm_logaction(handle, "ALPM-SCRIPTLET", "%s", line); EVENT(handle, &event); -- 2.6.4
On 01/02/16 at 10:14pm, Olivier Brunel wrote:
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@jjacky.com> --- Now always using the union member, to avoid initializer warnings (thanks Rikard) and better consistency. Hopefully this time it's all good.
lib/libalpm/add.c | 44 ++++++++++++++++++++++---------------------- lib/libalpm/be_sync.c | 6 +++--- lib/libalpm/handle.h | 2 +- lib/libalpm/hook.c | 22 +++++++++++----------- lib/libalpm/remove.c | 28 ++++++++++++++-------------- lib/libalpm/sync.c | 14 +++++++------- lib/libalpm/trans.c | 2 +- lib/libalpm/util.c | 6 +++--- 8 files changed, 62 insertions(+), 62 deletions(-)
Is there any reason not to just silence the warning by casting the event to (void*)? apg
On Mon, 4 Jan 2016 04:58:24 -0500 Andrew Gregory <andrew.gregory.8@gmail.com> wrote:
On 01/02/16 at 10:14pm, Olivier Brunel wrote:
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@jjacky.com> --- Now always using the union member, to avoid initializer warnings (thanks Rikard) and better consistency. Hopefully this time it's all good.
lib/libalpm/add.c | 44 ++++++++++++++++++++++---------------------- lib/libalpm/be_sync.c | 6 +++--- lib/libalpm/handle.h | 2 +- lib/libalpm/hook.c | 22 +++++++++++----------- lib/libalpm/remove.c | 28 ++++++++++++++-------------- lib/libalpm/sync.c | 14 +++++++------- lib/libalpm/trans.c | 2 +- lib/libalpm/util.c | 6 +++--- 8 files changed, 62 insertions(+), 62 deletions(-)
Is there any reason not to just silence the warning by casting the event to (void*)?
apg
hmm... I guess not. If that does silence the warning it would be a simpler "fix" indeed. Again I don't have clang so I can't test, but it should be enough indeed, so just changing the typecast in EVENT() should do it -- I assume a patch isn't needed then? -j
On 04/01/16 19:58, Andrew Gregory wrote:
On 01/02/16 at 10:14pm, Olivier Brunel wrote:
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@jjacky.com> --- Now always using the union member, to avoid initializer warnings (thanks Rikard) and better consistency. Hopefully this time it's all good.
lib/libalpm/add.c | 44 ++++++++++++++++++++++---------------------- lib/libalpm/be_sync.c | 6 +++--- lib/libalpm/handle.h | 2 +- lib/libalpm/hook.c | 22 +++++++++++----------- lib/libalpm/remove.c | 28 ++++++++++++++-------------- lib/libalpm/sync.c | 14 +++++++------- lib/libalpm/trans.c | 2 +- lib/libalpm/util.c | 6 +++--- 8 files changed, 62 insertions(+), 62 deletions(-)
Is there any reason not to just silence the warning by casting the event to (void*)?
Something like this? diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c index 49a5dae..ed79fdd 100644 --- a/lib/libalpm/hook.c +++ b/lib/libalpm/hook.c @@ -729,7 +729,7 @@ int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when) if(hooks_triggered != NULL) { event.type = ALPM_EVENT_HOOK_START; - EVENT(handle, &event); + EVENT(handle, (void *)&event); hook_event.position = 1; hook_event.total = triggered; @@ -758,7 +758,7 @@ int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when) alpm_list_free(hooks_triggered); event.type = ALPM_EVENT_HOOK_DONE; - EVENT(handle, &event); + EVENT(handle, (void *)&event); } cleanup: diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index 7e6d7bc..239d6a1 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -199,7 +199,7 @@ int SYMEXPORT alpm_trans_commit(alpm_handle_t *handle, alpm_list_t **data) alpm_logaction(handle, ALPM_CALLER_PREFIX, "transaction started\n"); event.type = ALPM_EVENT_TRANSACTION_START; - EVENT(handle, &event); + EVENT(handle, (void *)&event); if(trans->add == NULL) { if(_alpm_remove_packages(handle, 1) == -1) { @@ -223,7 +223,7 @@ int SYMEXPORT alpm_trans_commit(alpm_handle_t *handle, alpm_list_t **data) alpm_logaction(handle, ALPM_CALLER_PREFIX, "transaction interrupted\n"); } else { event.type = ALPM_EVENT_TRANSACTION_DONE; - EVENT(handle, &event); + EVENT(handle, (void *)&event); alpm_logaction(handle, ALPM_CALLER_PREFIX, "transaction completed\n"); _alpm_hook_run(handle, ALPM_HOOK_POST_TRANSACTION); }
participants (4)
-
Allan McRae
-
Andrew Gregory
-
Olivier Brunel
-
Rikard Falkeborn