[FFmpeg-devel] [RFC] Remove use of av_uninit

[FFmpeg-devel] [RFC] Remove use of av_uninit

[Zhao Zhili]

Background:

1. There is a av_uninit macro which suppress uninitialized variable warning

#if (defined(__GNUC__) || defined(__clang__)) && !defined(__INTEL_COMPILER)
#    define av_uninit(x) x=x
#else
#    define av_uninit(x) x
#endif

2. Declaration after statement coding style is allowed now

3. There is a crash fixed by ab792634197 which is related to av_uninit, and I have confronted it again recently

commit ab792634197e364ca1bb194f9abe36836e42f12d
Date:   Mon Oct 18 12:31:38 2021 +0300

    seek: Fix crashes in ff_seek_frame_binary if built with latest Clang 14

------------------------------------------------------------------------
What I suggests:

1. Deprecated av_uninit and removed it in the future

2. Remove usage in current code

3. When there is uninitiated variable warning

a. Declare variable when need if it’s possible, so it can be initiated at the same time
b. Use valid default value if there is
c. If there is no valid default value, or you think the variable never being used uninitialized, assign an insane value and add assert before use if

av_assert1(value != invalid_value);

Note:
1. I’m not suggesting always initialize variables
2. I’m suggesting remove av_uninit, until someone has an idea to make it more useful and less harmful.

Let’s ensure we stay on track and maintain focus on the central issue.

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [RFC] Remove use of av_uninit

[Nicolas George]

Zhao Zhili (HE12025-04-11):
> Background:
> 
> 1. There is a av_uninit macro which suppress uninitialized variable warning
> 
> #if (defined(__GNUC__) || defined(__clang__)) && !defined(__INTEL_COMPILER)
> #    define av_uninit(x) x=x
> #else
> #    define av_uninit(x) x
> #endif

Yes.

> 2. Declaration after statement coding style is allowed now

I fail to see the connection. Oh, reading further, I guess you think
moving the declaration of the variable will be enough to let the
compiler know the variable is not used and silence the warning. I
strongly doubt it might have that effect.

> 3. There is a crash fixed by ab792634197 which is related to av_uninit, and I have confronted it again recently

So, what does it prove? It proves that 16 years ago (e658657528d) Diego
silenced a warning without making sure the logic itself was sound.

Which is exactly what you are proposing to do right now in a different
way.

> 1. Deprecated av_uninit and removed it in the future
> 
> 2. Remove usage in current code

Not before or without 3.

> 3. When there is uninitiated variable warning
> 
> a. Declare variable when need if it’s possible, so it can be initiated at the same time
> b. Use valid default value if there is
> c. If there is no valid default value, or you think the variable never being used uninitialized, assign an insane value and add assert before use if
> 
> av_assert1(value != invalid_value);

Are you volunteering to do that huge work?

Running FATE with valgrind and no optimization seems like a quicker way
of finding the real bugs.

Regards,

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".