* [PATCH] videodev2.h: apply packed attribute to union
@ 2022-04-22 19:20 Devendra Tewari
2022-10-07 21:58 ` Kieran Bingham
0 siblings, 1 reply; 6+ messages in thread
From: Devendra Tewari @ 2022-04-22 19:20 UTC (permalink / raw)
To: linux-media; +Cc: Devendra Tewari
Fixes clang warning: field within 'v4l2_ext_control' is less than
'v4l2_ext_control::(anonymous union
Signed-off-by: Devendra Tewari <devendra.tewari@gmail.com>
---
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));
} __attribute__ ((packed));
struct v4l2_ext_controls {
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] videodev2.h: apply packed attribute to union
2022-04-22 19:20 [PATCH] videodev2.h: apply packed attribute to union Devendra Tewari
@ 2022-10-07 21:58 ` Kieran Bingham
2022-10-07 23:04 ` Devendra Tewari
0 siblings, 1 reply; 6+ messages in thread
From: Kieran Bingham @ 2022-10-07 21:58 UTC (permalink / raw)
To: Devendra Tewari, linux-media; +Cc: Devendra Tewari
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?
> 'v4l2_ext_control::(anonymous union
>
> Signed-off-by: Devendra Tewari <devendra.tewari@gmail.com>
> ---
> 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?
Is there any more context from the compiler warning beyond what is
reported above?
--
Kieran
> } __attribute__ ((packed));
>
> struct v4l2_ext_controls {
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] videodev2.h: apply packed attribute to union
2022-10-07 21:58 ` Kieran Bingham
@ 2022-10-07 23:04 ` Devendra Tewari
2022-10-08 11:04 ` Devendra Tewari
2022-10-08 11:48 ` Devendra Tewari
0 siblings, 2 replies; 6+ messages in thread
From: Devendra Tewari @ 2022-10-07 23:04 UTC (permalink / raw)
To: Kieran Bingham; +Cc: linux-media
> No dia 7 de out. de 2022, às 18:58, Kieran Bingham <kieran.bingham@ideasonboard.com> 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@gmail.com>
>> ---
>> 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.
>
> --
> Kieran
>
>
>> } __attribute__ ((packed));
>>
>> struct v4l2_ext_controls {
>> --
>> 2.25.1
>>
Thanks,
Devendra
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] videodev2.h: apply packed attribute to union
2022-10-07 23:04 ` Devendra Tewari
@ 2022-10-08 11:04 ` Devendra Tewari
2022-10-08 11:48 ` Devendra Tewari
1 sibling, 0 replies; 6+ messages in thread
From: Devendra Tewari @ 2022-10-08 11:04 UTC (permalink / raw)
To: Kieran Bingham; +Cc: linux-media
> On 7 Oct 2022, at 20:04, Devendra Tewari <devendra.tewari@gmail.com> wrote:
>
>
>
>> No dia 7 de out. de 2022, às 18:58, Kieran Bingham <kieran.bingham@ideasonboard.com> 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@gmail.com>
>>> ---
>>> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] videodev2.h: apply packed attribute to union
2022-10-07 23:04 ` Devendra Tewari
2022-10-08 11:04 ` Devendra Tewari
@ 2022-10-08 11:48 ` Devendra Tewari
2023-09-06 11:54 ` Hans Verkuil
1 sibling, 1 reply; 6+ messages in thread
From: Devendra Tewari @ 2022-10-08 11:48 UTC (permalink / raw)
To: Kieran Bingham; +Cc: linux-media
> On 7 Oct 2022, at 20:04, Devendra Tewari <devendra.tewari@gmail.com> wrote:
>
>
>
>> No dia 7 de out. de 2022, às 18:58, Kieran Bingham <kieran.bingham@ideasonboard.com> 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@gmail.com>
>>> ---
>>> 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.
The only explanation I can think of is that this may be the only instance where a union inside a packed struct has other structs that are not packed.
>
>> Is there any more context from the compiler warning beyond what is
>> reported above?
>
> I'll post a more detailed log asap.
>
>>
>> --
>> Kieran
>>
>>
>>> } __attribute__ ((packed));
>>>
>>> struct v4l2_ext_controls {
>>> --
>>> 2.25.1
>>>
> Thanks,
> Devendra
Resending because mail daemon rejected rich text message.
Thanks,
Devendra
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] videodev2.h: apply packed attribute to union
2022-10-08 11:48 ` Devendra Tewari
@ 2023-09-06 11:54 ` Hans Verkuil
0 siblings, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2023-09-06 11:54 UTC (permalink / raw)
To: Devendra Tewari, Kieran Bingham; +Cc: linux-media
Hi Devendra,
Cleaning up some old patches in
On 08/10/2022 13:48, Devendra Tewari wrote:
>
>
>> On 7 Oct 2022, at 20:04, Devendra Tewari <devendra.tewari@gmail.com> wrote:
>>
>>
>>
>>> No dia 7 de out. de 2022, às 18:58, Kieran Bingham <kieran.bingham@ideasonboard.com> 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@gmail.com>
>>>> ---
>>>> 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.
>
> The only explanation I can think of is that this may be the only instance where a union inside a packed struct has other structs that are not packed.
>
>>
>>> Is there any more context from the compiler warning beyond what is
>>> reported above?
>>
>> I'll post a more detailed log asap.
I haven't heard anything back, so I am rejecting this.
I am also afraid that changing this could cause unexpected ABI changes.
Regards,
Hans
>>
>>>
>>> --
>>> Kieran
>>>
>>>
>>>> } __attribute__ ((packed));
>>>>
>>>> struct v4l2_ext_controls {
>>>> --
>>>> 2.25.1
>>>>
>> Thanks,
>> Devendra
>
> Resending because mail daemon rejected rich text message.
>
> Thanks,
> Devendra
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-06 11:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-22 19:20 [PATCH] videodev2.h: apply packed attribute to union Devendra Tewari
2022-10-07 21:58 ` Kieran Bingham
2022-10-07 23:04 ` Devendra Tewari
2022-10-08 11:04 ` Devendra Tewari
2022-10-08 11:48 ` Devendra Tewari
2023-09-06 11:54 ` Hans Verkuil
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.