[FFmpeg-devel] [PATCH] avutil/utils: Remove racy check from avutil_version()

[James Almer]

On 9/26/2021 6:46 AM, Andreas Rheinhardt wrote:
> avutil_version() currently performs several checks before
> just returning the version. There is a static int that aims
> to ensure that these tests are run only once. The reason is that
> there used to be a slightly expensive check, but it has been removed
> in 92e3a6fdac73f7e1d69d69717219a7538877d7a0. Today running only
> once is unnecessary and can be counterproductive: GCC 10 optimizes
> all the actual checks away, but the checks_done variable and the code
> setting it has been kept. Given that this check is inherently racy
> (it uses non-atomic variables), it is best to just remove it.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> ---
> A part of me just wants to nuke all those checks.
> (We have zero callers of avutil_version() in the codebase and
> I don't see a reason why external users should call it more often,
> so all those checks probably don't fulfill their aim.)

We could replace them with C11's _Static_assert (Making sure that for 
C99 compilers like msvc it just becomes av_assert0). That way they will 
trigger at compilation time instead, which is more in line to what they 
were added for.

> 
>   libavutil/utils.c | 5 -----
>   1 file changed, 5 deletions(-)
> 
> diff --git a/libavutil/utils.c b/libavutil/utils.c
> index c1cd452eee..ea9b5097b8 100644
> --- a/libavutil/utils.c
> +++ b/libavutil/utils.c
> @@ -37,10 +37,6 @@ const char *av_version_info(void)
>   
>   unsigned avutil_version(void)
>   {
> -    static int checks_done;
> -    if (checks_done)
> -        return LIBAVUTIL_VERSION_INT;
> -
>       av_assert0(AV_SAMPLE_FMT_DBLP == 9);
>       av_assert0(AVMEDIA_TYPE_ATTACHMENT == 4);
>       av_assert0(AV_PICTURE_TYPE_BI == 7);
> @@ -58,7 +54,6 @@ unsigned avutil_version(void)
>           av_log(NULL, AV_LOG_ERROR, "Libavutil has been linked to a broken llrint()\n");
>       }
>   
> -    checks_done = 1;
>       return LIBAVUTIL_VERSION_INT;
>   }
>   
>