diff mbox

[FFmpeg-devel] lsws/utils: Split "emms_c(); " call in two lines

Message ID CAB0OVGpoZs9+2FsGzdGa7ehDATmSeBLxL3ryLzj=mjraDWoN5Q@mail.gmail.com
State Rejected
Headers show

Commit Message

Carl Eugen Hoyos Dec. 17, 2018, 1:30 p.m. UTC 
2018-12-17 13:24 GMT+01:00, Michael Niedermayer <michael@niedermayer.cc>:
> On Mon, Dec 17, 2018 at 02:14:26AM +0100, Carl Eugen Hoyos wrote:
>> Hi!
>>
>> Attached patch fixes a useless warning when compiling with clang on arm.
>>
>> Please comment, Carl Eugen
>
>>  utils.c |    3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> fb6799bbc59d79d9dfba9f8661aaecbed568f3a5
>> 0001-lsws-utils-Split-emms_c-in-two-lines.patch
>> From b2773d1ca9473380b99400c2bbd5a6729622f51a Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
>> Date: Mon, 17 Dec 2018 02:12:13 +0100
>> Subject: [PATCH] lsws/utils: Split "emms_c();" in two lines.
>>
>> Silences a clang warning when not compiling for x86:
>> libswscale/utils.c:345:13: warning: while loop has empty body
>> ---
>>  libswscale/utils.c |    3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/libswscale/utils.c b/libswscale/utils.c
>> index df68bcc..de64694 100644
>> --- a/libswscale/utils.c
>> +++ b/libswscale/utils.c
>> @@ -342,7 +342,8 @@ static av_cold int initFilter(int16_t **outFilter,
>> int32_t **filterPos,
>>      const int64_t fone = 1LL << (54 - FFMIN(av_log2(srcW/dstW), 8));
>>      int ret            = -1;
>>
>> -    emms_c(); // FIXME should not be required but IS (even for non-MMX
>> versions)
>> +    emms_c() // FIXME should not be required but IS (even for non-MMX
>> versions)
>> +        ;
>
> this looks like a bad change that could confuse people
> i think a different solution should be found or the warning
> left if theres none

New patch attached.

Please comment, Carl Eugen

Comments

Nicolas George Dec. 17, 2018, 3:45 p.m. UTC | #1 
Carl Eugen Hoyos (2018-12-17):
>  #ifndef emms_c
> -#   define emms_c() while(0)
> +#   define emms_c() while(0){}
>  #endif

That feels really wrong, or at least completely unusual. But not the
change you made, the original code: the usual statement is:
"do { statement } while (0)"

And if you make that change, you will get a warning about a semicolon
after a braced block.

I suggest to change the definition to

#define emms_c() do { } while (0)

Regards,
diff mbox

Patch

From 67f692d81792920f5344ceba239f64b2b9b4bec1 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Date: Mon, 17 Dec 2018 14:28:35 +0100
Subject: [PATCH] lavu/internal: Add an empty body to "while(0)".

Fixes a clang warning if arch != x86:
libswscale/utils.c:345:13: warning: while loop has empty body
---
 libavutil/internal.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavutil/internal.h b/libavutil/internal.h
index 06bd561..2a6e502 100644
--- a/libavutil/internal.h
+++ b/libavutil/internal.h
@@ -52,7 +52,7 @@ 
 #endif
 
 #ifndef emms_c
-#   define emms_c() while(0)
+#   define emms_c() while(0){}
 #endif
 
 #ifndef attribute_align_arg
-- 
1.7.10.4