[pacman-dev] Clang warnings

[Allan McRae]

On 02/01/16 02:46, Olivier Brunel wrote:
> 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.
> 

Use this style:

alpm_event_t event;
event.hook_run.type = ALPM_EVENT_HOOK_RUN_START;
...
EVENT(handle, &event)

Allan