[vlc-devel] [vlc-commits] src/missing: fix a bunch of clang compilation warnings and update copyright header

[Felix Paul Kühne]

On 03.03.2014, at 18:58, Rémi Denis-Courmont <remi at remlab.net> wrote:

> Le lundi 3 mars 2014, 18:50:52 Felix Paul Kühne a écrit :
>> Hey Rémi,
>> 
>> On 03.03.2014, at 18:42, Rémi Denis-Courmont <remi at remlab.net> wrote:
>>> Le lundi 3 mars 2014, 18:37:21 Felix Paul Kühne a écrit :
>>>> vlc | branch: master | Felix Paul Kühne <fkuehne at videolan.org> | Mon Mar 
>>>> 3
>>>> 18:28:42 2014 +0100| [374bd117c4390bcfbc1b56f40b1183560f256b0c] |
>>>> committer: Felix Paul Kühne
>>>> 
>>>> src/missing: fix a bunch of clang compilation warnings and update
>>>> copyright
>>>> header
>>> 
>>> There should be no warning on debug builds, since assert(0) is effectively
>>> a synonym for abort().
>> 
>> Well, this is true indeed. However, clang 5 seems to believe differently.
> 
> clang is right for the release case, but it is totally silly for the debug 
> case. Indeed, GCC does not warn about this here. There might be a "bug" in 
> Apple libc, failing to flag the assert failure function as non-returning.

Interesting :)

>> Here are 2 logs with the code state prior to my patch:
>> with --disable-debug: https://pastee.org/y79r5 (70 warnings)
>> with --enable-debug: https://pastee.org/jye6f (55 warnings)
>> 
>>> And now, there are a lot of unreachable code warnings…
>> 
>> Meh. I'll happily revert part of the patch (the return values) since the
>> remainder of unused variables already saves 55 warnings for me.
> 
> There are two problems here. The first problem is that there is a legitimate 
> warning in release (NDEBUG) build. To fix it, we would need a lot of ifdefs. I 
> am not sure this is worth the effort, as developers should use debug builds.
> 
> The second problem is that unreachable code and missing return value warnings 
> are fundamentally contradictory. We can absolutely not fix both, so for debug 
> builds, we have to choose whether we optimize for stupid or smart SDK's.

We could do #ifdef __llvm__ :p

I'll provide a patch which will remove "return after assert".

Cheers,

Felix