On 8/13/19 2:51 PM, Bart Van Assche wrote:
> On 8/13/19 1:25 PM, Jens Axboe wrote:
>> On 8/13/19 9:12 AM, Bart Van Assche wrote:
>>> On 8/12/19 7:53 PM, Jens Axboe wrote:
>>>> On 8/12/19 8:01 PM, Bart Van Assche wrote:
>>>>> Instead of declaring the whole structure packed, only declare non-aligned
>>>>> members packed. This patch is an alternative way to fix the following gcc 9
>>>>> compiler warnings:
>>>>>
>>>>> eta.c: In function 'calc_thread_status':
>>>>> eta.c:510:7: error: taking address of packed member of 'struct jobs_eta' may result in an unaligned pointer value [-Werror=address-of-packed-member]
>>>>> 510 | je->rate);
>>>>> | ~~^~~~~~
>>>>> eta.c:522:66: error: taking address of packed member of 'struct jobs_eta' may result in an unaligned pointer value [-Werror=address-of-packed-member]
>>>>> 522 | calc_rate(unified_rw_rep, disp_time, io_bytes, disp_io_bytes, je->rate);
>>>>> | ~~^~~~~~
>>>>> eta.c:523:64: error: taking address of packed member of 'struct jobs_eta' may result in an unaligned pointer value [-Werror=address-of-packed-member]
>>>>> 523 | calc_iops(unified_rw_rep, disp_time, io_iops, disp_io_iops, je->iops);
>>>>> |
>>>>
>>>> This seems fragile. Not that we change the struct all the time, or even often,
>>>> but it'd be easy to add members and end up with different layout on 32-bit
>>>> vs 64-bit.
>>>>
>>>> How do we improve on that?
>>>
>>> Hi Jens,
>>>
>>> Do you agree that the "BUILD_BUG_ON(sizeof(struct jobs_eta) != 160)"
>>> statement added by the previous patch should catch such differences? 160
>>> bytes namely is the size of an entirely packed jobs_eta structure.
>>
>> I guess that's good enough, though it'd be nice to check for holes
>> explicitly. I wonder if there's an easy way to do that, though...
>
> Hi Jens,
>
> The pahole tool should be able to check for holes. Unfortunately that
> tool doesn't seem to work very well on Ubuntu:
I was more thinking at build time. It'd be nice if you could do
something ala:
struct bla {
int a;
long b;
};
struct bla b1;
struct bla b2 __attribute__((__packed__));
BUILD_BUG_ON(sizeof(b1) != sizeof(b2));
If we rely on external components to do this, we might as well not do
it.
--
Jens Axboe
