[pacman-dev] Clang warnings

[Olivier Brunel]

On Fri, 1 Jan 2016 15:19:19 +0100
Rikard Falkeborn <rikard.falkeborn at gmail.com> wrote:

> Den 1 jan 2016 15:06 skrev "Olivier Brunel" <jjk at jjacky.com>:
> >
> > On Fri, 1 Jan 2016 23:03:22 +1000
> > Allan McRae <allan at 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 at 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