[FFmpeg-devel] avfilter/formats: fix wrong function name in error message

Submitted by Jun Zhao on Dec. 4, 2017, 5:02 a.m.

Details

Message ID c82a5728-9d97-2eb9-c471-2e106e772f00@gmail.com
State Accepted
Commit 4280948702bc256e21c375790b889c735d233b0d
Headers show

Commit Message

Jun Zhao Dec. 4, 2017, 5:02 a.m.
From 336be81bef86244a28b378b4c143ad8924c9e2d9 Mon Sep 17 00:00:00 2001
From: Jun Zhao <jun.zhao@intel.com>
Date: Mon, 4 Dec 2017 12:50:34 +0800
Subject: [PATCH] avfilter/formats: fix wrong function name in error message

Use perdefined micro __FUNCTION__ rather than hard coding function name
to fix wrong function name in error message.

Signed-off-by: Jun Zhao <jun.zhao@intel.com>
---
 libavfilter/formats.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Moritz Barsnick Dec. 4, 2017, 9:53 a.m.
On Mon, Dec 04, 2017 at 13:02:20 +0800, Jun Zhao wrote:
> Use perdefined micro __FUNCTION__ rather than hard coding function name
> to fix wrong function name in error message.

AFAICT, "__FUNCTION__" is a C99 feature and thereby not supported by
ffmpeg style. Or should it be? (It might be "supported by all compilers
we care about".)

http://ffmpeg.org/developer.html#C-language-features

Wondering,
Moritz
Hendrik Leppkes Dec. 4, 2017, 10:07 a.m.
On Mon, Dec 4, 2017 at 10:53 AM, Moritz Barsnick <barsnick@gmx.net> wrote:
> On Mon, Dec 04, 2017 at 13:02:20 +0800, Jun Zhao wrote:
>> Use perdefined micro __FUNCTION__ rather than hard coding function name
>> to fix wrong function name in error message.
>
> AFAICT, "__FUNCTION__" is a C99 feature and thereby not supported by
> ffmpeg style. Or should it be? (It might be "supported by all compilers
> we care about".)
>
> http://ffmpeg.org/developer.html#C-language-features
>

__FUNCTION__ is not C99, its a compiler extension (which is understood
by GCC and some other compilers). __func__ is the C99 keyword.
Its likely that not all compilers we currently support would have
__func__, if they all have __FUNCTION__ however I cannot tell.

- Hendrik
Michael Niedermayer Dec. 4, 2017, 6:32 p.m.
On Mon, Dec 04, 2017 at 11:07:11AM +0100, Hendrik Leppkes wrote:
> On Mon, Dec 4, 2017 at 10:53 AM, Moritz Barsnick <barsnick@gmx.net> wrote:
> > On Mon, Dec 04, 2017 at 13:02:20 +0800, Jun Zhao wrote:
> >> Use perdefined micro __FUNCTION__ rather than hard coding function name
> >> to fix wrong function name in error message.
> >
> > AFAICT, "__FUNCTION__" is a C99 feature and thereby not supported by
> > ffmpeg style. Or should it be? (It might be "supported by all compilers
> > we care about".)
> >
> > http://ffmpeg.org/developer.html#C-language-features
> >
> 
> __FUNCTION__ is not C99, its a compiler extension (which is understood
> by GCC and some other compilers). __func__ is the C99 keyword.
> Its likely that not all compilers we currently support would have
> __func__, if they all have __FUNCTION__ however I cannot tell.

There are some matches for __FUNCTION__ in git, so i wonder if all
compilers we support do support it

[...]
Jun Zhao Dec. 5, 2017, 5:42 a.m.
On 2017/12/5 2:32, Michael Niedermayer wrote:
> On Mon, Dec 04, 2017 at 11:07:11AM +0100, Hendrik Leppkes wrote:
>> On Mon, Dec 4, 2017 at 10:53 AM, Moritz Barsnick <barsnick@gmx.net> wrote:
>>> On Mon, Dec 04, 2017 at 13:02:20 +0800, Jun Zhao wrote:
>>>> Use perdefined micro __FUNCTION__ rather than hard coding function name
>>>> to fix wrong function name in error message.
>>> AFAICT, "__FUNCTION__" is a C99 feature and thereby not supported by
>>> ffmpeg style. Or should it be? (It might be "supported by all compilers
>>> we care about".)
>>>
>>> http://ffmpeg.org/developer.html#C-language-features
>>>
>> __FUNCTION__ is not C99, its a compiler extension (which is understood
>> by GCC and some other compilers). __func__ is the C99 keyword.
>> Its likely that not all compilers we currently support would have
>> __func__, if they all have __FUNCTION__ however I cannot tell.
> There are some matches for __FUNCTION__ in git, so i wonder if all
> compilers we support do support it
So now we have 2 option:

1). Find a ways to get the current function on different platforms in
C90,
https://stackoverflow.com/questions/7008485/func-or-function-or-manual-const-char-id
have some information for this way. (lot of #if defined )
2). Just remove __FUNCTION__ from the code. Total 5 __FUNCTION__ in
source code when grep the code.

Personally, I prefer option 2 than option 1, any comments?
>
> [...]
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Bang He Dec. 5, 2017, 9:20 a.m.
I prefer option 1). and microsoft vc++ uspport __FUNCTION__

On Tue, Dec 5, 2017 at 1:42 PM, Jun Zhao <mypopydev@gmail.com> wrote:

>
>
> On 2017/12/5 2:32, Michael Niedermayer wrote:
> > On Mon, Dec 04, 2017 at 11:07:11AM +0100, Hendrik Leppkes wrote:
> >> On Mon, Dec 4, 2017 at 10:53 AM, Moritz Barsnick <barsnick@gmx.net>
> wrote:
> >>> On Mon, Dec 04, 2017 at 13:02:20 +0800, Jun Zhao wrote:
> >>>> Use perdefined micro __FUNCTION__ rather than hard coding function
> name
> >>>> to fix wrong function name in error message.
> >>> AFAICT, "__FUNCTION__" is a C99 feature and thereby not supported by
> >>> ffmpeg style. Or should it be? (It might be "supported by all compilers
> >>> we care about".)
> >>>
> >>> http://ffmpeg.org/developer.html#C-language-features
> >>>
> >> __FUNCTION__ is not C99, its a compiler extension (which is understood
> >> by GCC and some other compilers). __func__ is the C99 keyword.
> >> Its likely that not all compilers we currently support would have
> >> __func__, if they all have __FUNCTION__ however I cannot tell.
> > There are some matches for __FUNCTION__ in git, so i wonder if all
> > compilers we support do support it
> So now we have 2 option:
>
> 1). Find a ways to get the current function on different platforms in
> C90,
> https://stackoverflow.com/questions/7008485/func-or-
> function-or-manual-const-char-id
> have some information for this way. (lot of #if defined )
> 2). Just remove __FUNCTION__ from the code. Total 5 __FUNCTION__ in
> source code when grep the code.
>
> Personally, I prefer option 2 than option 1, any comments?
> >
> > [...]
> >
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Hendrik Leppkes Dec. 5, 2017, 9:25 a.m.
On Tue, Dec 5, 2017 at 6:42 AM, Jun Zhao <mypopydev@gmail.com> wrote:
>
>
> On 2017/12/5 2:32, Michael Niedermayer wrote:
>> On Mon, Dec 04, 2017 at 11:07:11AM +0100, Hendrik Leppkes wrote:
>>> On Mon, Dec 4, 2017 at 10:53 AM, Moritz Barsnick <barsnick@gmx.net> wrote:
>>>> On Mon, Dec 04, 2017 at 13:02:20 +0800, Jun Zhao wrote:
>>>>> Use perdefined micro __FUNCTION__ rather than hard coding function name
>>>>> to fix wrong function name in error message.
>>>> AFAICT, "__FUNCTION__" is a C99 feature and thereby not supported by
>>>> ffmpeg style. Or should it be? (It might be "supported by all compilers
>>>> we care about".)
>>>>
>>>> http://ffmpeg.org/developer.html#C-language-features
>>>>
>>> __FUNCTION__ is not C99, its a compiler extension (which is understood
>>> by GCC and some other compilers). __func__ is the C99 keyword.
>>> Its likely that not all compilers we currently support would have
>>> __func__, if they all have __FUNCTION__ however I cannot tell.
>> There are some matches for __FUNCTION__ in git, so i wonder if all
>> compilers we support do support it
> So now we have 2 option:
>
> 1). Find a ways to get the current function on different platforms in
> C90,
> https://stackoverflow.com/questions/7008485/func-or-function-or-manual-const-char-id
> have some information for this way. (lot of #if defined )
> 2). Just remove __FUNCTION__ from the code. Total 5 __FUNCTION__ in
> source code when grep the code.
>
> Personally, I prefer option 2 than option 1, any comments?


If __FUNCTION__ is already in use right now (and hence supported by
all compilers we have), it should be fine to use it again, so no need
for complex ifdefs, I would think.
In fact I just checked, and its in use in a key part in avformat, not
even an optional module, so any compiler not supporting it would
already fail building it now.

- Hendrik
Jun Zhao Dec. 7, 2017, 12:49 a.m.
On 2017/12/5 17:25, Hendrik Leppkes wrote:
> On Tue, Dec 5, 2017 at 6:42 AM, Jun Zhao <mypopydev@gmail.com> wrote:
>>
>> On 2017/12/5 2:32, Michael Niedermayer wrote:
>>> On Mon, Dec 04, 2017 at 11:07:11AM +0100, Hendrik Leppkes wrote:
>>>> On Mon, Dec 4, 2017 at 10:53 AM, Moritz Barsnick <barsnick@gmx.net> wrote:
>>>>> On Mon, Dec 04, 2017 at 13:02:20 +0800, Jun Zhao wrote:
>>>>>> Use perdefined micro __FUNCTION__ rather than hard coding function name
>>>>>> to fix wrong function name in error message.
>>>>> AFAICT, "__FUNCTION__" is a C99 feature and thereby not supported by
>>>>> ffmpeg style. Or should it be? (It might be "supported by all compilers
>>>>> we care about".)
>>>>>
>>>>> http://ffmpeg.org/developer.html#C-language-features
>>>>>
>>>> __FUNCTION__ is not C99, its a compiler extension (which is understood
>>>> by GCC and some other compilers). __func__ is the C99 keyword.
>>>> Its likely that not all compilers we currently support would have
>>>> __func__, if they all have __FUNCTION__ however I cannot tell.
>>> There are some matches for __FUNCTION__ in git, so i wonder if all
>>> compilers we support do support it
>> So now we have 2 option:
>>
>> 1). Find a ways to get the current function on different platforms in
>> C90,
>> https://stackoverflow.com/questions/7008485/func-or-function-or-manual-const-char-id
>> have some information for this way. (lot of #if defined )
>> 2). Just remove __FUNCTION__ from the code. Total 5 __FUNCTION__ in
>> source code when grep the code.
>>
>> Personally, I prefer option 2 than option 1, any comments?
>
> If __FUNCTION__ is already in use right now (and hence supported by
> all compilers we have), it should be fine to use it again, so no need
> for complex ifdefs, I would think.
> In fact I just checked, and its in use in a key part in avformat, not
> even an optional module, so any compiler not supporting it would
> already fail building it now.
>
> - Hendrik
I agree, so we need to wait more comments for this ?
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Jun Zhao Dec. 12, 2017, 5:59 a.m.
On 2017/12/7 8:49, Jun Zhao wrote:
>
> On 2017/12/5 17:25, Hendrik Leppkes wrote:
>> On Tue, Dec 5, 2017 at 6:42 AM, Jun Zhao <mypopydev@gmail.com> wrote:
>>> On 2017/12/5 2:32, Michael Niedermayer wrote:
>>>> On Mon, Dec 04, 2017 at 11:07:11AM +0100, Hendrik Leppkes wrote:
>>>>> On Mon, Dec 4, 2017 at 10:53 AM, Moritz Barsnick <barsnick@gmx.net> wrote:
>>>>>> On Mon, Dec 04, 2017 at 13:02:20 +0800, Jun Zhao wrote:
>>>>>>> Use perdefined micro __FUNCTION__ rather than hard coding function name
>>>>>>> to fix wrong function name in error message.
>>>>>> AFAICT, "__FUNCTION__" is a C99 feature and thereby not supported by
>>>>>> ffmpeg style. Or should it be? (It might be "supported by all compilers
>>>>>> we care about".)
>>>>>>
>>>>>> http://ffmpeg.org/developer.html#C-language-features
>>>>>>
>>>>> __FUNCTION__ is not C99, its a compiler extension (which is understood
>>>>> by GCC and some other compilers). __func__ is the C99 keyword.
>>>>> Its likely that not all compilers we currently support would have
>>>>> __func__, if they all have __FUNCTION__ however I cannot tell.
>>>> There are some matches for __FUNCTION__ in git, so i wonder if all
>>>> compilers we support do support it
>>> So now we have 2 option:
>>>
>>> 1). Find a ways to get the current function on different platforms in
>>> C90,
>>> https://stackoverflow.com/questions/7008485/func-or-function-or-manual-const-char-id
>>> have some information for this way. (lot of #if defined )
>>> 2). Just remove __FUNCTION__ from the code. Total 5 __FUNCTION__ in
>>> source code when grep the code.
>>>
>>> Personally, I prefer option 2 than option 1, any comments?
>> If __FUNCTION__ is already in use right now (and hence supported by
>> all compilers we have), it should be fine to use it again, so no need
>> for complex ifdefs, I would think.
>> In fact I just checked, and its in use in a key part in avformat, not
>> even an optional module, so any compiler not supporting it would
>> already fail building it now.
>>
>> - Hendrik
> I agree, so we need to wait more comments for this ?

Any comments, Michael ? or we need to remove all __FUNCTION__ from the
code ?
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Michael Niedermayer Dec. 12, 2017, 5:12 p.m.
On Tue, Dec 12, 2017 at 01:59:18PM +0800, Jun Zhao wrote:
> 
> 
> On 2017/12/7 8:49, Jun Zhao wrote:
> >
> > On 2017/12/5 17:25, Hendrik Leppkes wrote:
> >> On Tue, Dec 5, 2017 at 6:42 AM, Jun Zhao <mypopydev@gmail.com> wrote:
> >>> On 2017/12/5 2:32, Michael Niedermayer wrote:
> >>>> On Mon, Dec 04, 2017 at 11:07:11AM +0100, Hendrik Leppkes wrote:
> >>>>> On Mon, Dec 4, 2017 at 10:53 AM, Moritz Barsnick <barsnick@gmx.net> wrote:
> >>>>>> On Mon, Dec 04, 2017 at 13:02:20 +0800, Jun Zhao wrote:
> >>>>>>> Use perdefined micro __FUNCTION__ rather than hard coding function name
> >>>>>>> to fix wrong function name in error message.
> >>>>>> AFAICT, "__FUNCTION__" is a C99 feature and thereby not supported by
> >>>>>> ffmpeg style. Or should it be? (It might be "supported by all compilers
> >>>>>> we care about".)
> >>>>>>
> >>>>>> http://ffmpeg.org/developer.html#C-language-features
> >>>>>>
> >>>>> __FUNCTION__ is not C99, its a compiler extension (which is understood
> >>>>> by GCC and some other compilers). __func__ is the C99 keyword.
> >>>>> Its likely that not all compilers we currently support would have
> >>>>> __func__, if they all have __FUNCTION__ however I cannot tell.
> >>>> There are some matches for __FUNCTION__ in git, so i wonder if all
> >>>> compilers we support do support it
> >>> So now we have 2 option:
> >>>
> >>> 1). Find a ways to get the current function on different platforms in
> >>> C90,
> >>> https://stackoverflow.com/questions/7008485/func-or-function-or-manual-const-char-id
> >>> have some information for this way. (lot of #if defined )
> >>> 2). Just remove __FUNCTION__ from the code. Total 5 __FUNCTION__ in
> >>> source code when grep the code.
> >>>
> >>> Personally, I prefer option 2 than option 1, any comments?
> >> If __FUNCTION__ is already in use right now (and hence supported by
> >> all compilers we have), it should be fine to use it again, so no need
> >> for complex ifdefs, I would think.
> >> In fact I just checked, and its in use in a key part in avformat, not
> >> even an optional module, so any compiler not supporting it would
> >> already fail building it now.
> >>
> >> - Hendrik
> > I agree, so we need to wait more comments for this ?
> 
> Any comments, Michael ? or we need to remove all __FUNCTION__ from the
> code ?

ill apply the patch as it seems all concerns have been resolved IIUC

thx

[...]

Patch hide | download patch | download mbox

diff --git a/libavfilter/formats.c b/libavfilter/formats.c
index d4de862237..20a2c89719 100644
--- a/libavfilter/formats.c
+++ b/libavfilter/formats.c
@@ -72,7 +72,7 @@  do {
             for (j = 0; j < b->nb; j++)                                         \
                 if (a->fmts[i] == b->fmts[j]) {                                 \
                     if(k >= FFMIN(a->nb, b->nb)){                               \
-                        av_log(NULL, AV_LOG_ERROR, "Duplicate formats in avfilter_merge_formats() detected\n"); \
+                        av_log(NULL, AV_LOG_ERROR, "Duplicate formats in %s detected\n", __FUNCTION__); \
                         av_free(ret->fmts);                                     \
                         av_free(ret);                                           \
                         return NULL;                                            \