Re: [Qemu-devel] [PATCH 1/2] tcg-arm: Implement tcg_register_jit

[Andreas Färber]

Am 05.06.2013 14:49, schrieb Richard Henderson:
> On 06/04/2013 06:56 PM, li guang wrote:
>>>> +typedef struct {
>>>> +    uint32_t len __attribute__((aligned((sizeof(void *)))));
>>>> +    uint32_t cie_offset;
>>>> +    tcg_target_long func_start __attribute__((packed));
>>>> +    tcg_target_long func_len __attribute__((packed));
>> suspicious usage of packed attribute here,
>> since tcg_targe_long is either 32 or 64 bits,
>> not a struct or union.
>>
>> Thanks!
>>
> 
> Your question is worded poorly -- what has struct/union got to do with it?  
> One
> can adjust the alignment of any type.  Perhaps you don't know what it is that
> __attribute__((packed)) actually does?

To me the English word "packed" refers to a struct containing no
alignment padding, i.e. sizeof(the struct) = sum(sizeof(each field)).
The use of __attribute__((packed)) on an individual field while quite
possibly valid is unusual and I believe we have a QEMU_PACKED macro.

So why can't you apply QEMU_PACKED to the whole struct? Because of the
contradicting void* alignment attribute of the first field?

Cheers,
Andreas

> While it's true that for ARM all four of these data members are 32-bit, and
> thus none of the attributes are required, it's not actually wrong.  Given that
> this sort of boiler-plate tends to get copied from target to target, and since
> the attributes *are* required for 64-bit hosts, I prefer to keep all such
> structures defined similarly.
> 
> 
> r~

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg