diff mbox series

[FFmpeg-devel] lavu/video_enc_params: Avoid relying on an undefined C construct

Message ID 20230115224741.3446965-1-martin@martin.st
State Accepted
Commit 01f58f36465a13db069641cca47fa9777f03b875
Headers show
Series [FFmpeg-devel] lavu/video_enc_params: Avoid relying on an undefined C construct | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished

Commit Message

Martin Storsjö Jan. 15, 2023, 10:47 p.m. UTC
The construct of using offsetof on a (potentially anonymous) struct
defined within the offsetof expression, while supported by all
current compilers, has been declared explicitly undefined by the
C standards committee [1].

Current Clang git main got a patch [2] which changed this construct
into a hard error. It seems like this will be softened into a
warning [3] soon though, as it did break a fair number of projects.

Nevertheless - in this particular case, it's trivial to fix the
code not to rely on the construct that the standards committee has
explicitly called out as undefined.

[1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm
[2] https://github.com/llvm/llvm-project/commit/e327b52766ed497e4779f4e652b9ad237dfda8e6
[3] https://reviews.llvm.org/D133574#4053647

Signed-off-by: Martin Storsjö <martin@martin.st>
---
 libavutil/video_enc_params.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Anton Khirnov Jan. 18, 2023, 9:38 a.m. UTC | #1
Quoting Martin Storsjö (2023-01-15 23:47:41)
> The construct of using offsetof on a (potentially anonymous) struct
> defined within the offsetof expression, while supported by all
> current compilers, has been declared explicitly undefined by the
> C standards committee [1].
> 
> Current Clang git main got a patch [2] which changed this construct
> into a hard error. It seems like this will be softened into a
> warning [3] soon though, as it did break a fair number of projects.
> 
> Nevertheless - in this particular case, it's trivial to fix the
> code not to rely on the construct that the standards committee has
> explicitly called out as undefined.
> 
> [1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm
> [2] https://github.com/llvm/llvm-project/commit/e327b52766ed497e4779f4e652b9ad237dfda8e6
> [3] https://reviews.llvm.org/D133574#4053647
> 
> Signed-off-by: Martin Storsjö <martin@martin.st>
> ---
>  libavutil/video_enc_params.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/libavutil/video_enc_params.c b/libavutil/video_enc_params.c
> index 54bfed0ed9..33592dc128 100644
> --- a/libavutil/video_enc_params.c
> +++ b/libavutil/video_enc_params.c
> @@ -27,11 +27,11 @@
>  AVVideoEncParams *av_video_enc_params_alloc(enum AVVideoEncParamsType type,
>                                              unsigned int nb_blocks, size_t *out_size)
>  {
> -    const size_t blocks_offset = offsetof(
> -        struct {
> -            AVVideoEncParams   p;
> -            AVVideoBlockParams b;
> -        }, b);
> +    struct TestStruct {
> +        AVVideoEncParams   p;
> +        AVVideoBlockParams b;
> +    };
> +    const size_t blocks_offset = offsetof(struct TestStruct, b);
>      size_t size = blocks_offset;
>      AVVideoEncParams *par;
>  
> -- 
> 2.34.1

LGTM
Martin Storsjö Jan. 31, 2023, 12:37 p.m. UTC | #2
On Wed, 18 Jan 2023, Anton Khirnov wrote:

> Quoting Martin Storsjö (2023-01-15 23:47:41)
>> The construct of using offsetof on a (potentially anonymous) struct
>> defined within the offsetof expression, while supported by all
>> current compilers, has been declared explicitly undefined by the
>> C standards committee [1].
>>
>> Current Clang git main got a patch [2] which changed this construct
>> into a hard error. It seems like this will be softened into a
>> warning [3] soon though, as it did break a fair number of projects.
>>
>> Nevertheless - in this particular case, it's trivial to fix the
>> code not to rely on the construct that the standards committee has
>> explicitly called out as undefined.
>>
>> [1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm
>> [2] https://github.com/llvm/llvm-project/commit/e327b52766ed497e4779f4e652b9ad237dfda8e6
>> [3] https://reviews.llvm.org/D133574#4053647
>>
>> Signed-off-by: Martin Storsjö <martin@martin.st>
>> ---
>>  libavutil/video_enc_params.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavutil/video_enc_params.c b/libavutil/video_enc_params.c
>> index 54bfed0ed9..33592dc128 100644
>> --- a/libavutil/video_enc_params.c
>> +++ b/libavutil/video_enc_params.c
>> @@ -27,11 +27,11 @@
>>  AVVideoEncParams *av_video_enc_params_alloc(enum AVVideoEncParamsType type,
>>                                              unsigned int nb_blocks, size_t *out_size)
>>  {
>> -    const size_t blocks_offset = offsetof(
>> -        struct {
>> -            AVVideoEncParams   p;
>> -            AVVideoBlockParams b;
>> -        }, b);
>> +    struct TestStruct {
>> +        AVVideoEncParams   p;
>> +        AVVideoBlockParams b;
>> +    };
>> +    const size_t blocks_offset = offsetof(struct TestStruct, b);
>>      size_t size = blocks_offset;
>>      AVVideoEncParams *par;
>>
>> --
>> 2.34.1
>
> LGTM

Thanks, pushed now.

// Martin
diff mbox series

Patch

diff --git a/libavutil/video_enc_params.c b/libavutil/video_enc_params.c
index 54bfed0ed9..33592dc128 100644
--- a/libavutil/video_enc_params.c
+++ b/libavutil/video_enc_params.c
@@ -27,11 +27,11 @@ 
 AVVideoEncParams *av_video_enc_params_alloc(enum AVVideoEncParamsType type,
                                             unsigned int nb_blocks, size_t *out_size)
 {
-    const size_t blocks_offset = offsetof(
-        struct {
-            AVVideoEncParams   p;
-            AVVideoBlockParams b;
-        }, b);
+    struct TestStruct {
+        AVVideoEncParams   p;
+        AVVideoBlockParams b;
+    };
+    const size_t blocks_offset = offsetof(struct TestStruct, b);
     size_t size = blocks_offset;
     AVVideoEncParams *par;