[vlc-devel] [PATCH] contrib: sidplay2: fix signed char stored as int8_t

[Rémi Denis-Courmont]

Hi,

Narrowing is not an error. And the patch does not even fix the narrowing problem on platforms with signed 'char' such as x86. Looks like some bad -Werror here.

Le 9 janvier 2020 20:49:09 GMT+09:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>This is the error I get:
>
>/bin/bash ../../libtool  --tag=CXX   --mode=compile 
>arm-linux-gnueabihf-g++ -DHAVE_CONFIG_H 
>-I/mnt/c/Users/robux/Documents/Programs/Videolabs/build/rasp/contrib/arm-linux-gnueabihf/include
>
> 
>-I/mnt/c/Users/robux/Documents/Programs/Videolabs/build/rasp/contrib/arm-linux-gnueabihf/include
>
>  -g -O2 -DHAVE_UNIX -I../../include -I../../include/sidplay -c -o 
>xsid.lo xsid.cpp
>libtool: compile:  arm-linux-gnueabihf-g++ -DHAVE_CONFIG_H 
>-I/mnt/c/Users/robux/Documents/Programs/Videolabs/build/rasp/contrib/arm-linux-gnueabihf/include
>
>-I/mnt/c/Users/robux/Documents/Programs/Videolabs/build/rasp/contrib/arm-linux-gnueabihf/include
>
>-g -O2 -DHAVE_UNIX -I../../include -I../../include/sidplay -c xsid.cpp 
>-fPIC -DPIC -o xsid.o
>xsid.cpp:101:1: error: narrowing conversion of ‘'\200'’ from ‘char’ to 
>‘int8_t’ {aka ‘signed char’} [-Wnarrowing]
>   101 | };
>       | ^
>xsid.cpp:101:1: error: narrowing conversion of ‘'\224'’ from ‘char’ to 
>‘int8_t’ {aka ‘signed char’} [-Wnarrowing]
>xsid.cpp:101:1: error: narrowing conversion of ‘'\251'’ from ‘char’ to 
>‘int8_t’ {aka ‘signed char’} [-Wnarrowing]
>xsid.cpp:101:1: error: narrowing conversion of ‘'\274'’ from ‘char’ to 
>‘int8_t’ {aka ‘signed char’} [-Wnarrowing]
>xsid.cpp:101:1: error: narrowing conversion of ‘'\316'’ from ‘char’ to 
>‘int8_t’ {aka ‘signed char’} [-Wnarrowing]
>xsid.cpp:101:1: error: narrowing conversion of ‘'\341'’ from ‘char’ to 
>‘int8_t’ {aka ‘signed char’} [-Wnarrowing]
>xsid.cpp:101:1: error: narrowing conversion of ‘'\362'’ from ‘char’ to 
>‘int8_t’ {aka ‘signed char’} [-Wnarrowing]
>make[4]: *** [Makefile:409: xsid.lo] Error 1
>
>On 2020-01-09 10:54, Martin Storsjö wrote:
>> On Thu, 9 Jan 2020, Steve Lhomme wrote:
>> 
>>> Fixes compilation with gcc 9 for Raspberry
>>> ---
>>> contrib/src/sidplay2/rules.mak                |  1 +
>>> contrib/src/sidplay2/sidplay2-char-cast.patch | 22
>+++++++++++++++++++
>>> 2 files changed, 23 insertions(+)
>>> create mode 100644 contrib/src/sidplay2/sidplay2-char-cast.patch
>> 
>> Um, can you quote the actual error you're fixing here? Because 
>> intuitively to me, the current state seems much more correct.
>> 
>>>
>>> diff --git a/contrib/src/sidplay2/rules.mak 
>>> b/contrib/src/sidplay2/rules.mak
>>> index 84c35ccfee7..de77935774a 100644
>>> --- a/contrib/src/sidplay2/rules.mak
>>> +++ b/contrib/src/sidplay2/rules.mak
>>> @@ -25,6 +25,7 @@ sidplay-libs: sidplay-libs-$(SID_VERSION).tar.gz 
>>> .sum-sidplay2
>>>     $(APPLY) $(SRC)/sidplay2/sidplay2-string.patch
>>>     $(APPLY) $(SRC)/sidplay2/sidplay-fix-ln-s.patch
>>>     $(APPLY) $(SRC)/sidplay2/sidplay2-resid-dependency.patch
>>> +    $(APPLY) $(SRC)/sidplay2/sidplay2-char-cast.patch
>>>     $(MOVE)
>>>
>>> .sidplay2: sidplay-libs
>>> diff --git a/contrib/src/sidplay2/sidplay2-char-cast.patch 
>>> b/contrib/src/sidplay2/sidplay2-char-cast.patch
>>> new file mode 100644
>>> index 00000000000..a16898945fa
>>> --- /dev/null
>>> +++ b/contrib/src/sidplay2/sidplay2-char-cast.patch
>>> @@ -0,0 +1,22 @@
>>> +--- sidplay-libs/libsidplay/src/xsid/xsid.cpp.char    2020-01-08 
>>> 14:09:08.124096200 +0100
>>> ++++ sidplay-libs/libsidplay/src/xsid/xsid.cpp    2020-01-08 
>>> 14:09:16.204111900 +0100
>>> +@@ -94,7 +94,7 @@ const int8_t XSID::sampleConvertTable[16
>>> +     '\x08', '\x19', '\x2a', '\x3b', '\x4c', '\x5d', '\x6e', '\x7f'
>>> + };
>>> + */
>>> +-const int8_t XSID::sampleConvertTable[16] =
>>> ++const char XSID::sampleConvertTable[16] =
>>> + {
>>> +     '\x80', '\x94', '\xa9', '\xbc', '\xce', '\xe1', '\xf2',
>'\x03',
>>> +     '\x1b', '\x2a', '\x3b', '\x49', '\x58', '\x66', '\x73', '\x7f'
>> 
>> Using 'char' here is probably wrong, as the plain 'char' type has 
>> different signedness on different platforms.
>> 
>> Is the problem that the table contains values that are intended to be
>
>> interpreted as signed, but written in the table as unsigned values?
>> 
>> // Martin
>> 
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200109/bb273a7e/attachment.html>