[FFmpeg-devel] libavutil/thread.h: Fixed g++ build error when ASSERT_LEVEL is greater than 1

[Marton Balint]

On Thu, 20 Apr 2017, Aaron Levinson wrote:

> On 4/19/2017 2:27 PM, Marton Balint wrote:
>>
>> On Mon, 17 Apr 2017, James Almer wrote:
>>
>>> On 4/17/2017 5:39 AM, Clément Bœsch wrote:
>>>> On Sun, Apr 16, 2017 at 05:20:02PM -0700, Aaron Levinson wrote:
>>>>> From 9e6a9e2b8d58f17c661a3f455e03c95587ec7b18 Mon Sep 17 00:00:00 2001
>>>>> From: Aaron Levinson <alevinsn at aracnet.com>
>>>>> Date: Sun, 16 Apr 2017 17:13:31 -0700
>>>>> Subject: [PATCH] libavutil/thread.h:  Fixed g++ build error when
>>>>>  ASSERT_LEVEL is greater than 1
>>>>>
>>>>> Purpose: libavutil/thread.h: Fixed g++ build error when ASSERT_LEVEL
>>>>> is greater than 1.  This is only relevant when thread.h is included by
>>>>> C++ files.  In this case, the relevant code is only defined if
>>>>> HAVE_PTHREADS is defined as 1.  Use configure --assert-level=2 to do
>>>>> so.
>>>>>
>>>>> Note: Issue discovered as a result of Coverity build failure.  Cause
>>>>> of build failure pinpointed by Hendrik Leppkes.
>>>>>
>>>>> Comments:
>>>>>
>>>>> -- libavutil/thread.h: Altered ASSERT_PTHREAD_NORET definition such
>>>>>    that it uses av_make_error_string instead of av_err2str().
>>>>>    av_err2str() uses a "parenthesized type followed by an initializer
>>>>>    list", which is apparently not valid C++.  This issue started
>>>>>    occurring because thread.h is now included by the DeckLink C++
>>>>>    files.  The alteration does the equivalent of what av_err2str()
>>>>>    does, but instead declares the character buffer as a local
>>>>>    variable.
>>>>> ---
>>>>>  libavutil/thread.h | 5 ++++-
>>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/libavutil/thread.h b/libavutil/thread.h
>>>>> index 6e57447..f108e20 100644
>>>>> --- a/libavutil/thread.h
>>>>> +++ b/libavutil/thread.h
>>>>> @@ -36,8 +36,11 @@
>>>>>  #define ASSERT_PTHREAD_NORET(func, ...) do
>>>>> {                            \
>>>>>      int ret =
>>>>> func(__VA_ARGS__);                                        \
>>>>>      if (ret)
>>>>> {                                                          \
>>>>> +        char errbuf[AV_ERROR_MAX_STRING_SIZE] =
>>>>> "";                     \
>>>>>          av_log(NULL, AV_LOG_FATAL,
>>>>> AV_STRINGIFY(func)                   \
>>>>> -               " failed with error: %s\n",
>>>>> av_err2str(AVERROR(ret)));   \
>>>>> +               " failed with error:
>>>>> %s\n",                              \
>>>>> +               av_make_error_string(errbuf,
>>>>> AV_ERROR_MAX_STRING_SIZE,   \
>>>>> +
>>>>> AVERROR(ret)));                     \
>>>>>
>>>>> abort();                                                        \
>>>>>
>>>>> }                                                                   \
>>>>>  } while (0)
>>>>
>>>> I don't like limiting ourselves in the common C code of the project
>>>> because C++ is a bad and limited language. Can't you solve this by
>>>> bumping
>>>> the minimal requirement of C++ version?
>>>
>>> We're already using C++11 when available because of atomics on
>>> mediacodec.
>>> Also, just tried and it seems to fail even with C++14, so it just doesn't
>>> work with C++.
>>>
>>> We could instead just make these strict assert wrappers work only on C
>>> code by for example checking for defined(__cplusplus).
>>
>> I'd say let's apply the patch as is, that is the simplest solution.
>>
>> Regards,
>> Marton
>
> I don't think most people care one way or the other--perhaps someone 
> with push privileges could apply the patch?  I assume that the Coverity 
> builds are continuing to fail as a result of the issue that this patch 
> addresses.

Ok, I will apply this tomorrow.

Regards,
Marton