Bug 642626 – =media-libs/opencollada-1.6.58 fails to build wue to -Werror on -funsigned-char platforms: COLLADABUURI.cpp:57:2: error: narrowing conversion of '-1' from 'int' to 'char' inside { } [-Wnarrowing]

• Status: RESOLVED FIXED
• Product: Gentoo Linux

[Description Sergei Trofimovich (RETIRED) 2017-12-29 12:08:22 UTC]

Created attachment 511964 [details]
media-libs-opencollada-1.6.58-build.log

Found the failure on ppc64.

To reproduce it on x86/amd64 you can pass -funsigned-char explicitly:
    CXXFLAGS=-funsigned-char emerge -v1 media-libs/opencollada

[Comment 1 Sergei Trofimovich (RETIRED) 2017-12-29 12:10:42 UTC]

media-libs/opencollada-1.6.58/work/OpenCOLLADA-1.6.58/COLLADABaseUtils/src/COLLADABUURI.cpp:57:2: error: narrowing conversion of '-1' from 'int' to 'char' inside { } [-Wnarro
wing]                                                                                                                                                                                          
  };                                                                                                                                                                                           
  ^ 

Caused by the following code:

    const char HEX2DEC[256] =
    {
        /*       0  1  2  3   4  5  6  7   8  9  A  B   C  D  E  F */
        /* 0 */ -1,-1,-1,-1, -1,-1,-1,-1, -1,-1,-1,-1, -1,-1,-1,-1,
        /* 1 */ -1,-1,-1,-1, -1,-1,-1,-1, -1,-1,-1,-1, -1,-1,-1,-1,
        /* 2 */ -1,-1,-1,-1, -1,-1,-1,-1, -1,-1,-1,-1, -1,-1,-1,-1,
        /* 3 */  0, 1, 2, 3,  4, 5, 6, 7,  8, 9,-1,-1, -1,-1,-1,-1,

'char' is unsigned on powerpc and powerpc64.

[Comment 2 Jonathan Scruggs (RETIRED) 2017-12-29 13:49:38 UTC]

I was looking at supported variables for PowerPC, and found "signed char". If I changed it to that explicit type, would it compile then?

[Comment 3 Sergei Trofimovich (RETIRED) 2017-12-29 22:13:36 UTC]

The following at least survives 'CXXFLAGS=-funsigned-char emerge -v1 media-libs/opencollad'

--- a/COLLADABaseUtils/src/COLLADABUURI.cpp
+++ b/COLLADABaseUtils/src/COLLADABUURI.cpp
@@ -34,3 +34,3 @@ namespace COLLADABU

-       const char HEX2DEC[256] =
+       const signed char HEX2DEC[256] = 
        {
@@ -110,4 +110,4 @@ namespace COLLADABU
                                char dec1, dec2;
-                               if (    (char)(-1) != (dec1 = HEX2DEC[*(pSrc + 1)])
-                                    && (char)(-1) != (dec2 = HEX2DEC[*(pSrc + 2)]))
+                               if (    (signed char)(-1) != (dec1 = HEX2DEC[*(pSrc + 1)])
+                                    && (signed char)(-1) != (dec2 = HEX2DEC[*(pSrc + 2)]))
                                {

[Comment 4 Jonathan Scruggs (RETIRED) 2017-12-29 22:33:53 UTC]

I added a few sed lines to the ebuild that do the same thing as the patch. It compiled on x86_64 architecture, but I need to see if it works right. I'll ask upstream what they think of the changes.

[Comment 5 Andreas Sturmlechner 2017-12-29 22:57:30 UTC]

Please use a patch rather than sed for these code changes. Since you are going to approach upstream anyway...

[Comment 6 Jonathan Scruggs (RETIRED) 2017-12-30 03:15:41 UTC]

I only did that to make a quick test ebuild, not a final product.

[Comment 7 Jonathan Scruggs (RETIRED) 2018-01-13 10:04:18 UTC]

Sergei Trofimovich,

The patch was accepted upstream, https://github.com/KhronosGroup/OpenCOLLADA/pull/559, and version 1.62 was released with this fix. Do you think the stable on the PPC arches should be held off for the stable of 1.62? The difference between 1.58 and 1.61 is all Windows fixes, which is why I wasn't releasing those. Then 1.62 is only the signed char fix. Upstream is very fast at releasing fixes.

I think we should try to fast track 1.62 to stabilisation, especially on the PPC arches since somewhere along the way, they broke compatibility.

Regards,
Jon

[Comment 8 Sergei Trofimovich (RETIRED) 2018-01-13 12:05:43 UTC]

(In reply to Jonathan Scruggs from comment #7)
> Sergei Trofimovich,
> 
> The patch was accepted upstream,
> https://github.com/KhronosGroup/OpenCOLLADA/pull/559, and version 1.62 was
> released with this fix. Do you think the stable on the PPC arches should be
> held off for the stable of 1.62? The difference between 1.58 and 1.61 is all
> Windows fixes, which is why I wasn't releasing those. Then 1.62 is only the
> signed char fix. Upstream is very fast at releasing fixes.
> 
> I think we should try to fast track 1.62 to stabilisation, especially on the
> PPC arches since somewhere along the way, they broke compatibility.
> 
> Regards,
> Jon

Sounds good.

[Comment 9 Jonathan Scruggs (RETIRED) 2018-01-31 17:04:38 UTC]

I released 1.62 yesterday. I forgot to link it to this bug, but it was commit number 65833f7dc77f342cfbd705d2548cc76f32de2d8e, so I'll close this bug now. :) Hope it works for everyone now.