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

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

[Andrei Borzenkov]

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;
 }
 
-- 
tg: (7a21030..) u/xfs-clang-align (depends on: master)

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

[Leif Lindholm]

On Fri, Jul 03, 2015 at 10:05:47PM +0300, 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?

Well, the problem is that struct grub_xfs_btree_node is defined with
GRUB_PACKED - forcing a 1-byte alignment requirement as opposed to the
8-byte requirement that would naturally be enforced by the struct
contents. And apparently clang objects to this, whereas gcc thinks
everything is fine ... even though -Wcast-align is explicitly used.

Now, grub_xfs_btree_keys() is only called by grub_xfs_read_block(),
where it is immediately stuffed back into another 8-byte aligned
pointer. And then the alignment is immediately discarded again by
casting it to a (char *) for an arithmetic operation.

If the alignment is indeed not required, it may be worth explicitly
marking that pointer as one to a potentially unaligned location.
But we don't currently appear to have a GRUB_UNALIGNED macro, to match
the GRUB_PACKED for structs. Should we?

If so, something like the following could be added to your patch for a
more complete fix:
--- a/grub-core/fs/xfs.c
+++ b/grub-core/fs/xfs.c
@@ -488,7 +488,7 @@ grub_xfs_read_block (grub_fshelp_node_t node,
grub_disk_addr_t fileblock)
   if (node->inode.format == XFS_INODE_FORMAT_BTREE)
     {
       struct grub_xfs_btree_root *root;
-      const grub_uint64_t *keys;
+      const grub_uint64_t *keys GRUB_UNALIGNED;
       int recoffset;
 
       leaf = grub_malloc (node->data->bsize);
diff --git a/include/grub/types.h b/include/grub/types.h
index e732efb..720e236 100644
--- a/include/grub/types.h
+++ b/include/grub/types.h
@@ -30,6 +30,8 @@
 #define GRUB_PACKED __attribute__ ((packed))
 #endif
 
+#define GRUB_UNALIGNED __attribute__ ((aligned (1)))
+
 #ifdef GRUB_BUILD
 # define GRUB_CPU_SIZEOF_VOID_P        BUILD_SIZEOF_VOID_P
 # define GRUB_CPU_SIZEOF_LONG  BUILD_SIZEOF_LONG

/
    Leif

>  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;
>  }
>  
> -- 
> tg: (7a21030..) u/xfs-clang-align (depends on: master)
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

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

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

Go ahead
On 07.07.2015 19:17, Leif Lindholm wrote:
> On Fri, Jul 03, 2015 at 10:05:47PM +0300, 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?
> 
> Well, the problem is that struct grub_xfs_btree_node is defined with
> GRUB_PACKED - forcing a 1-byte alignment requirement as opposed to the
> 8-byte requirement that would naturally be enforced by the struct
> contents. And apparently clang objects to this, whereas gcc thinks
> everything is fine ... even though -Wcast-align is explicitly used.
> 
> Now, grub_xfs_btree_keys() is only called by grub_xfs_read_block(),
> where it is immediately stuffed back into another 8-byte aligned
> pointer. And then the alignment is immediately discarded again by
> casting it to a (char *) for an arithmetic operation.
> 
> If the alignment is indeed not required, it may be worth explicitly
> marking that pointer as one to a potentially unaligned location.
> But we don't currently appear to have a GRUB_UNALIGNED macro, to match
> the GRUB_PACKED for structs. Should we?
> 
> If so, something like the following could be added to your patch for a
> more complete fix:
> --- a/grub-core/fs/xfs.c
> +++ b/grub-core/fs/xfs.c
> @@ -488,7 +488,7 @@ grub_xfs_read_block (grub_fshelp_node_t node,
> grub_disk_addr_t fileblock)
>    if (node->inode.format == XFS_INODE_FORMAT_BTREE)
>      {
>        struct grub_xfs_btree_root *root;
> -      const grub_uint64_t *keys;
> +      const grub_uint64_t *keys GRUB_UNALIGNED;
>        int recoffset;
>  
>        leaf = grub_malloc (node->data->bsize);
> diff --git a/include/grub/types.h b/include/grub/types.h
> index e732efb..720e236 100644
> --- a/include/grub/types.h
> +++ b/include/grub/types.h
> @@ -30,6 +30,8 @@
>  #define GRUB_PACKED __attribute__ ((packed))
>  #endif
>  
> +#define GRUB_UNALIGNED __attribute__ ((aligned (1)))
> +
>  #ifdef GRUB_BUILD
>  # define GRUB_CPU_SIZEOF_VOID_P        BUILD_SIZEOF_VOID_P
>  # define GRUB_CPU_SIZEOF_LONG  BUILD_SIZEOF_LONG
> 
> /
>     Leif
> 
>>  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;
>>  }
>>  
>> -- 
>> tg: (7a21030..) u/xfs-clang-align (depends on: master)
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



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

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

[Leif Lindholm]

On Wed, Jul 15, 2015 at 05:47:50PM +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> Go ahead

The below was more of an RFC than something committable - are you OK
with me splitting the types.h changes out as a separate patch?

> On 07.07.2015 19:17, Leif Lindholm wrote:
> > On Fri, Jul 03, 2015 at 10:05:47PM +0300, 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?
> > 
> > Well, the problem is that struct grub_xfs_btree_node is defined with
> > GRUB_PACKED - forcing a 1-byte alignment requirement as opposed to the
> > 8-byte requirement that would naturally be enforced by the struct
> > contents. And apparently clang objects to this, whereas gcc thinks
> > everything is fine ... even though -Wcast-align is explicitly used.
> > 
> > Now, grub_xfs_btree_keys() is only called by grub_xfs_read_block(),
> > where it is immediately stuffed back into another 8-byte aligned
> > pointer. And then the alignment is immediately discarded again by
> > casting it to a (char *) for an arithmetic operation.
> > 
> > If the alignment is indeed not required, it may be worth explicitly
> > marking that pointer as one to a potentially unaligned location.
> > But we don't currently appear to have a GRUB_UNALIGNED macro, to match
> > the GRUB_PACKED for structs. Should we?
> > 
> > If so, something like the following could be added to your patch for a
> > more complete fix:
> > --- a/grub-core/fs/xfs.c
> > +++ b/grub-core/fs/xfs.c
> > @@ -488,7 +488,7 @@ grub_xfs_read_block (grub_fshelp_node_t node,
> > grub_disk_addr_t fileblock)
> >    if (node->inode.format == XFS_INODE_FORMAT_BTREE)
> >      {
> >        struct grub_xfs_btree_root *root;
> > -      const grub_uint64_t *keys;
> > +      const grub_uint64_t *keys GRUB_UNALIGNED;
> >        int recoffset;
> >  
> >        leaf = grub_malloc (node->data->bsize);
> > diff --git a/include/grub/types.h b/include/grub/types.h
> > index e732efb..720e236 100644
> > --- a/include/grub/types.h
> > +++ b/include/grub/types.h
> > @@ -30,6 +30,8 @@
> >  #define GRUB_PACKED __attribute__ ((packed))
> >  #endif
> >  
> > +#define GRUB_UNALIGNED __attribute__ ((aligned (1)))
> > +
> >  #ifdef GRUB_BUILD
> >  # define GRUB_CPU_SIZEOF_VOID_P        BUILD_SIZEOF_VOID_P
> >  # define GRUB_CPU_SIZEOF_LONG  BUILD_SIZEOF_LONG
> > 
> > /
> >     Leif
> > 
> >>  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;
> >>  }
> >>  
> >> -- 
> >> tg: (7a21030..) u/xfs-clang-align (depends on: master)
> >>
> >> _______________________________________________
> >> Grub-devel mailing list
> >> Grub-devel@gnu.org
> >> https://lists.gnu.org/mailman/listinfo/grub-devel
> > 
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> > 
> 
> 



> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

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

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 15.07.2015 18:52, Leif Lindholm wrote:
> On Wed, Jul 15, 2015 at 05:47:50PM +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> Go ahead
> 
> The below was more of an RFC than something committable - are you OK
> with me splitting the types.h changes out as a separate patch?
> 
Yes
>> On 07.07.2015 19:17, Leif Lindholm wrote:
>>> On Fri, Jul 03, 2015 at 10:05:47PM +0300, 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?
>>>
>>> Well, the problem is that struct grub_xfs_btree_node is defined with
>>> GRUB_PACKED - forcing a 1-byte alignment requirement as opposed to the
>>> 8-byte requirement that would naturally be enforced by the struct
>>> contents. And apparently clang objects to this, whereas gcc thinks
>>> everything is fine ... even though -Wcast-align is explicitly used.
>>>
>>> Now, grub_xfs_btree_keys() is only called by grub_xfs_read_block(),
>>> where it is immediately stuffed back into another 8-byte aligned
>>> pointer. And then the alignment is immediately discarded again by
>>> casting it to a (char *) for an arithmetic operation.
>>>
>>> If the alignment is indeed not required, it may be worth explicitly
>>> marking that pointer as one to a potentially unaligned location.
>>> But we don't currently appear to have a GRUB_UNALIGNED macro, to match
>>> the GRUB_PACKED for structs. Should we?
>>>
>>> If so, something like the following could be added to your patch for a
>>> more complete fix:
>>> --- a/grub-core/fs/xfs.c
>>> +++ b/grub-core/fs/xfs.c
>>> @@ -488,7 +488,7 @@ grub_xfs_read_block (grub_fshelp_node_t node,
>>> grub_disk_addr_t fileblock)
>>>    if (node->inode.format == XFS_INODE_FORMAT_BTREE)
>>>      {
>>>        struct grub_xfs_btree_root *root;
>>> -      const grub_uint64_t *keys;
>>> +      const grub_uint64_t *keys GRUB_UNALIGNED;
>>>        int recoffset;
>>>  
>>>        leaf = grub_malloc (node->data->bsize);
>>> diff --git a/include/grub/types.h b/include/grub/types.h
>>> index e732efb..720e236 100644
>>> --- a/include/grub/types.h
>>> +++ b/include/grub/types.h
>>> @@ -30,6 +30,8 @@
>>>  #define GRUB_PACKED __attribute__ ((packed))
>>>  #endif
>>>  
>>> +#define GRUB_UNALIGNED __attribute__ ((aligned (1)))
>>> +
>>>  #ifdef GRUB_BUILD
>>>  # define GRUB_CPU_SIZEOF_VOID_P        BUILD_SIZEOF_VOID_P
>>>  # define GRUB_CPU_SIZEOF_LONG  BUILD_SIZEOF_LONG
>>>
>>> /
>>>     Leif
>>>
>>>>  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;
>>>>  }
>>>>  
>>>> -- 
>>>> tg: (7a21030..) u/xfs-clang-align (depends on: master)
>>>>
>>>> _______________________________________________
>>>> Grub-devel mailing list
>>>> Grub-devel@gnu.org
>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>
>>
> 
> 
> 
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



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

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

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 15.07.2015 18:52, Leif Lindholm wrote:
> On Wed, Jul 15, 2015 at 05:47:50PM +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> Go ahead
> 
> The below was more of an RFC than something committable - are you OK
> with me splitting the types.h changes out as a separate patch?
> 
Actually I think this patch doesn't work as __attribute__((aligned(X)))
can only increase alignment, not decrease it. I'm going to fix it by
using char * and grub_get_unaligned64
>> On 07.07.2015 19:17, Leif Lindholm wrote:
>>> On Fri, Jul 03, 2015 at 10:05:47PM +0300, 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?
>>>
>>> Well, the problem is that struct grub_xfs_btree_node is defined with
>>> GRUB_PACKED - forcing a 1-byte alignment requirement as opposed to the
>>> 8-byte requirement that would naturally be enforced by the struct
>>> contents. And apparently clang objects to this, whereas gcc thinks
>>> everything is fine ... even though -Wcast-align is explicitly used.
>>>
>>> Now, grub_xfs_btree_keys() is only called by grub_xfs_read_block(),
>>> where it is immediately stuffed back into another 8-byte aligned
>>> pointer. And then the alignment is immediately discarded again by
>>> casting it to a (char *) for an arithmetic operation.
>>>
>>> If the alignment is indeed not required, it may be worth explicitly
>>> marking that pointer as one to a potentially unaligned location.
>>> But we don't currently appear to have a GRUB_UNALIGNED macro, to match
>>> the GRUB_PACKED for structs. Should we?
>>>
>>> If so, something like the following could be added to your patch for a
>>> more complete fix:
>>> --- a/grub-core/fs/xfs.c
>>> +++ b/grub-core/fs/xfs.c
>>> @@ -488,7 +488,7 @@ grub_xfs_read_block (grub_fshelp_node_t node,
>>> grub_disk_addr_t fileblock)
>>>    if (node->inode.format == XFS_INODE_FORMAT_BTREE)
>>>      {
>>>        struct grub_xfs_btree_root *root;
>>> -      const grub_uint64_t *keys;
>>> +      const grub_uint64_t *keys GRUB_UNALIGNED;
>>>        int recoffset;
>>>  
>>>        leaf = grub_malloc (node->data->bsize);
>>> diff --git a/include/grub/types.h b/include/grub/types.h
>>> index e732efb..720e236 100644
>>> --- a/include/grub/types.h
>>> +++ b/include/grub/types.h
>>> @@ -30,6 +30,8 @@
>>>  #define GRUB_PACKED __attribute__ ((packed))
>>>  #endif
>>>  
>>> +#define GRUB_UNALIGNED __attribute__ ((aligned (1)))
>>> +
>>>  #ifdef GRUB_BUILD
>>>  # define GRUB_CPU_SIZEOF_VOID_P        BUILD_SIZEOF_VOID_P
>>>  # define GRUB_CPU_SIZEOF_LONG  BUILD_SIZEOF_LONG
>>>
>>> /
>>>     Leif
>>>
>>>>  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;
>>>>  }
>>>>  
>>>> -- 
>>>> tg: (7a21030..) u/xfs-clang-align (depends on: master)
>>>>
>>>> _______________________________________________
>>>> Grub-devel mailing list
>>>> Grub-devel@gnu.org
>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>
>>
> 
> 
> 
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



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

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

[Leif Lindholm]

On Thu, Jul 16, 2015 at 12:07:14PM +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 15.07.2015 18:52, Leif Lindholm wrote:
> > On Wed, Jul 15, 2015 at 05:47:50PM +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> >> Go ahead
> > 
> > The below was more of an RFC than something committable - are you OK
> > with me splitting the types.h changes out as a separate patch?
> > 
> Actually I think this patch doesn't work as __attribute__((aligned(X)))
> can only increase alignment, not decrease it. I'm going to fix it by
> using char * and grub_get_unaligned64

Yeah, you're right - it should actually have been (packed) again.

Anyway, you already contributed a fix.

/
    Leif

> >> On 07.07.2015 19:17, Leif Lindholm wrote:
> >>> On Fri, Jul 03, 2015 at 10:05:47PM +0300, 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?
> >>>
> >>> Well, the problem is that struct grub_xfs_btree_node is defined with
> >>> GRUB_PACKED - forcing a 1-byte alignment requirement as opposed to the
> >>> 8-byte requirement that would naturally be enforced by the struct
> >>> contents. And apparently clang objects to this, whereas gcc thinks
> >>> everything is fine ... even though -Wcast-align is explicitly used.
> >>>
> >>> Now, grub_xfs_btree_keys() is only called by grub_xfs_read_block(),
> >>> where it is immediately stuffed back into another 8-byte aligned
> >>> pointer. And then the alignment is immediately discarded again by
> >>> casting it to a (char *) for an arithmetic operation.
> >>>
> >>> If the alignment is indeed not required, it may be worth explicitly
> >>> marking that pointer as one to a potentially unaligned location.
> >>> But we don't currently appear to have a GRUB_UNALIGNED macro, to match
> >>> the GRUB_PACKED for structs. Should we?
> >>>
> >>> If so, something like the following could be added to your patch for a
> >>> more complete fix:
> >>> --- a/grub-core/fs/xfs.c
> >>> +++ b/grub-core/fs/xfs.c
> >>> @@ -488,7 +488,7 @@ grub_xfs_read_block (grub_fshelp_node_t node,
> >>> grub_disk_addr_t fileblock)
> >>>    if (node->inode.format == XFS_INODE_FORMAT_BTREE)
> >>>      {
> >>>        struct grub_xfs_btree_root *root;
> >>> -      const grub_uint64_t *keys;
> >>> +      const grub_uint64_t *keys GRUB_UNALIGNED;
> >>>        int recoffset;
> >>>  
> >>>        leaf = grub_malloc (node->data->bsize);
> >>> diff --git a/include/grub/types.h b/include/grub/types.h
> >>> index e732efb..720e236 100644
> >>> --- a/include/grub/types.h
> >>> +++ b/include/grub/types.h
> >>> @@ -30,6 +30,8 @@
> >>>  #define GRUB_PACKED __attribute__ ((packed))
> >>>  #endif
> >>>  
> >>> +#define GRUB_UNALIGNED __attribute__ ((aligned (1)))
> >>> +
> >>>  #ifdef GRUB_BUILD
> >>>  # define GRUB_CPU_SIZEOF_VOID_P        BUILD_SIZEOF_VOID_P
> >>>  # define GRUB_CPU_SIZEOF_LONG  BUILD_SIZEOF_LONG
> >>>
> >>> /
> >>>     Leif
> >>>
> >>>>  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;
> >>>>  }
> >>>>  
> >>>> -- 
> >>>> tg: (7a21030..) u/xfs-clang-align (depends on: master)
> >>>>
> >>>> _______________________________________________
> >>>> Grub-devel mailing list
> >>>> Grub-devel@gnu.org
> >>>> https://lists.gnu.org/mailman/listinfo/grub-devel
> >>>
> >>> _______________________________________________
> >>> Grub-devel mailing list
> >>> Grub-devel@gnu.org
> >>> https://lists.gnu.org/mailman/listinfo/grub-devel
> >>>
> >>
> >>
> > 
> > 
> > 
> >> _______________________________________________
> >> Grub-devel mailing list
> >> Grub-devel@gnu.org
> >> https://lists.gnu.org/mailman/listinfo/grub-devel
> > 
> > 
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> > 
> 
> 



> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

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

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 15.07.2015 18:52, Leif Lindholm wrote:
> On Wed, Jul 15, 2015 at 05:47:50PM +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> Go ahead
> 
> The below was more of an RFC than something committable - are you OK
> with me splitting the types.h changes out as a separate patch?
> 
I've fixed this and several other bad alignments in xfs.c
>> On 07.07.2015 19:17, Leif Lindholm wrote:
>>> On Fri, Jul 03, 2015 at 10:05:47PM +0300, 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?
>>>
>>> Well, the problem is that struct grub_xfs_btree_node is defined with
>>> GRUB_PACKED - forcing a 1-byte alignment requirement as opposed to the
>>> 8-byte requirement that would naturally be enforced by the struct
>>> contents. And apparently clang objects to this, whereas gcc thinks
>>> everything is fine ... even though -Wcast-align is explicitly used.
>>>
>>> Now, grub_xfs_btree_keys() is only called by grub_xfs_read_block(),
>>> where it is immediately stuffed back into another 8-byte aligned
>>> pointer. And then the alignment is immediately discarded again by
>>> casting it to a (char *) for an arithmetic operation.
>>>
>>> If the alignment is indeed not required, it may be worth explicitly
>>> marking that pointer as one to a potentially unaligned location.
>>> But we don't currently appear to have a GRUB_UNALIGNED macro, to match
>>> the GRUB_PACKED for structs. Should we?
>>>
>>> If so, something like the following could be added to your patch for a
>>> more complete fix:
>>> --- a/grub-core/fs/xfs.c
>>> +++ b/grub-core/fs/xfs.c
>>> @@ -488,7 +488,7 @@ grub_xfs_read_block (grub_fshelp_node_t node,
>>> grub_disk_addr_t fileblock)
>>>    if (node->inode.format == XFS_INODE_FORMAT_BTREE)
>>>      {
>>>        struct grub_xfs_btree_root *root;
>>> -      const grub_uint64_t *keys;
>>> +      const grub_uint64_t *keys GRUB_UNALIGNED;
>>>        int recoffset;
>>>  
>>>        leaf = grub_malloc (node->data->bsize);
>>> diff --git a/include/grub/types.h b/include/grub/types.h
>>> index e732efb..720e236 100644
>>> --- a/include/grub/types.h
>>> +++ b/include/grub/types.h
>>> @@ -30,6 +30,8 @@
>>>  #define GRUB_PACKED __attribute__ ((packed))
>>>  #endif
>>>  
>>> +#define GRUB_UNALIGNED __attribute__ ((aligned (1)))
>>> +
>>>  #ifdef GRUB_BUILD
>>>  # define GRUB_CPU_SIZEOF_VOID_P        BUILD_SIZEOF_VOID_P
>>>  # define GRUB_CPU_SIZEOF_LONG  BUILD_SIZEOF_LONG
>>>
>>> /
>>>     Leif
>>>
>>>>  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;
>>>>  }
>>>>  
>>>> -- 
>>>> tg: (7a21030..) u/xfs-clang-align (depends on: master)
>>>>
>>>> _______________________________________________
>>>> Grub-devel mailing list
>>>> Grub-devel@gnu.org
>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>
>>
> 
> 
> 
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



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

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

[Vladimir 'φ-coder/phcoder' Serbinenko]

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


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



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

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

[Andrei Borzenkov]

[-- 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 --]

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

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 16.07.2015 05:46, Andrei Borzenkov wrote:
> В 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?
> 
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.


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