Re: [PATCH] zfs: fix compilation failure with clang due to alignment

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 16.07.2015 05:46, Andrei Borzenkov wrote:
> В Wed, 15 Jul 2015 17:49:30 +0200
> Vladimir 'φ-coder/phcoder' Serbinenko <address@hidden> пишет:
> 
>>
>> On 03.07.2015 21:05, Andrei Borzenkov wrote:
>>> I do not claim I understand why clang complains, but this patch does
>>> fix it.
>>>
>>> fs/xfs.c:452:25: error: cast from 'struct grub_xfs_btree_node *' to
>>>       'grub_uint64_t *' (aka 'unsigned long long *') increases required
>>>       alignment from 1 to 8 [-Werror,-Wcast-align]
>>>   grub_uint64_t *keys = (grub_uint64_t *)(leaf + 1);
>>>                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> 1 error generated.
>>>
>>> ---
>>>
>>> Jan, do you have any idea what's wrong and whether this is proper fix?
>>> Or should I raise it with clang?
>>>
>>>  grub-core/fs/xfs.c | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
>>> index 7249291..ea8cf7e 100644
>>> --- a/grub-core/fs/xfs.c
>>> +++ b/grub-core/fs/xfs.c
>>> @@ -445,14 +445,14 @@ grub_xfs_next_de(struct grub_xfs_data *data, struct 
>>> grub_xfs_dir2_entry *de)
>>>    return (struct grub_xfs_dir2_entry *)(((char *)de) + ALIGN_UP(size, 8));
>>>  }
>>>  
>>> -static grub_uint64_t *
>>> +static void *
>>>  grub_xfs_btree_keys(struct grub_xfs_data *data,
>>>                 struct grub_xfs_btree_node *leaf)
>>>  {
>>> -  grub_uint64_t *keys = (grub_uint64_t *)(leaf + 1);
>>> +  char *keys = (char *)leaf + sizeof (*leaf);
>>>  
>>>    if (data->hascrc)
>>> -    keys += 6;     /* skip crc, uuid, ... */
>>> +    keys += 6 * sizeof (grub_uint64_t);    /* skip crc, uuid, ... */
>>>    return keys;
>>>  }
>>>  
>> This would only hide the problem behind void*. Leif's patch solves the
>> problem. Another possibility is to analyze if packed is really required
>> but most likely it is.
>>>
>>
>>
> 
> grub_uint64_t is not really required at all. The whole code is used to
> compute bite offset. So in reality this should simply be replaced by
> char *. I would rather avoid making code even more complicated.
> 
> Jan, do I miss something?
> 
We have exactly the same problem at line 500:
      keys = &root->keys[0];
and root is in packed struct. I think it's better to solve both in the
same way.

signature.asc
Description: OpenPGP digital signature