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

[Andrei Borzenkov <arvidjaar@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2073 bytes --]

В Wed, 15 Jul 2015 17:49:30 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> 
> 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?

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]