Re: [PATCH] videodev2.h: apply packed attribute to union

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On 7 Oct 2022, at 20:04, Devendra Tewari <devendra.tewari@xxxxxxxxx> wrote:
> 
> 
> 
>> No dia 7 de out. de 2022, às 18:58, Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx> escreveu:
>> 
>> Hi Devendra,
>> 
>> Quoting Devendra Tewari (2022-04-22 20:20:31)
>>> Fixes clang warning: field within 'v4l2_ext_control' is less than
>> 
>> Can you detail which version of clang this occurs with? Have you tried
>> more than one version?
>> 
> 
> This started happening with version 14.0.1 and I continue to see it with version 15.
> 
>> 
>>> 'v4l2_ext_control::(anonymous union
>>> 
>>> Signed-off-by: Devendra Tewari <devendra.tewari@xxxxxxxxx>
>>> ---
>>> include/uapi/linux/videodev2.h | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>> index 3768a0a80830..767c52c722cd 100644
>>> --- a/include/uapi/linux/videodev2.h
>>> +++ b/include/uapi/linux/videodev2.h
>>> @@ -1765,7 +1765,7 @@ struct v4l2_ext_control {
>>>               struct v4l2_ctrl_vp9_compressed_hdr __user *p_vp9_compressed_hdr_probs;
>>>               struct v4l2_ctrl_vp9_frame __user *p_vp9_frame;
>>>               void __user *ptr;
>>> -       };
>>> +       } __attribute__ ((packed));
>> 
>> This is a curious fix. It's applying a packed attribute to the union,
>> which I presume means that it's then applying the packed attribute to
>> any item in the union.
>> 
>> The items are all either: __s32, __s64, values - or pointers.
>> 
>> While applying this attribute here may fix the compiler warning, I'm not
>> sure it's clear why this is required. This file also has other
>> locations where a union inside a packed struct is not marked as packed.
>> Should all unions be marked with the attribute?
> 
> Interesting - I need to look deeper into packed.
> 
>> Is there any more context from the compiler warning beyond what is
>> reported above?
> 
> I'll post a more detailed log asap.

This is the log with clang 15 compiler…

 ../git/src/libcamera/v4l2_device.cpp
| In file included from ../git/src/libcamera/v4l2_device.cpp:8:
| In file included from ../git/include/libcamera/internal/v4l2_device.h:15:
| ../git/include/linux/videodev2.h:1724:2: error: field  within 'v4l2_ext_control' is less aligned than 'v4l2_ext_control::(anonymous union at ../git/include/linux/videodev2.h:1724:2)' and is usually due to 'v4l2_ext_control' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
|         union {
|         ^
| 1 error generated.


Compiler version...

# clang --version
clang version 15.0.1 (https://github.com/llvm/llvm-project 29d395a1b7a8176abb1d6278f7df98301fbe7744)
Target: x86_64-unknown-linux-gnu
Thread model: posix

> 
>> 
>> --
>> Kieran
>> 
>> 
>>> } __attribute__ ((packed));
>>> 
>>> struct v4l2_ext_controls {
>>> -- 
>>> 2.25.1
>>> 
> Thanks,
> Devendra
Resending because mailing list daemon is unhappy with rich text.

Thanks,
Devendra





[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux