Compiliation fails on ARM platforms (invalid type conversion)

[newbluemoon]

Compilation for ARM platforms using gcc 7.2 fails with invalid conversion from 'signed char*' to 'const char*'. I believe this is because on ARM the char data type is unsigned by default, but in src/formats/pngformat.cpp and src/formats/yasaraformat.cpp negative values are assigned. I used Open Babel version 2.4.1 but it’s still the same in the latest source files.

The following patch which replaces the negative integer with its two’s complement works, but I have no idea if that breaks something else. The best solution would probably be to use singed char.

--- src/formats/pngformat.cpp.orig      2016-09-21 21:55:37.000000000 +0200
+++ src/formats/pngformat.cpp   2017-11-10 08:17:28.625719045 +0100
@@ -218,7 +218,7 @@
     _count=0;
     _hasInputPngFile=true;
   }
-  const char pngheader[] = {-119,80,78,71,13,10,26,10,0};
+  const char pngheader[] = {0x9,80,78,71,13,10,26,10,0};
   char readbytes[9];
   ifs.read(readbytes, 8);

--- src/formats/yasaraformat.cpp.orig   2016-09-21 21:55:37.000000000 +0200
+++ src/formats/yasaraformat.cpp        2017-11-10 08:07:02.620775489 +0100
@@ -472,7 +472,7 @@

   //  bool hetatom;
   char buffer[32],/*resname[4],*/atomname[5];
-  char double1[8]={0,0,0,0,0,0,-16,0x3f};
+  char double1[8]={0,0,0,0,0,0,0x10,0x3f};
   //   char *str;
   int i,j,/*m,q,*/pos;
   int /*resno,chainNum,link,linktype,*/atoms,element,links/*,chain*/;

[baoilleach]

Thanks. I think it's not the two's complement, but 256-val, as negative values underflow. (Two's complement is the representation in assembly, as far as I know.) However, I'd like to look at the code first and see why the negative value was used in the first place.

[baoilleach]

How are you doing the compilation? Is that cross-compilation from a different platform for example? Which ARM device exactly are you using/targeting? I have an old Raspberry Pi that I can possibly use to test.

The reason I ask is that while the compilation error can be made to disappear, I would still be worried that the other uses of char in those functions may be silently affected and so I'd like to test.

[newbluemoon]

I cross-compiled it on x86_64 for armv7l using Void Linux’s package building tool xbps-src. Void Linux cross compiles all packages for armv6l, armv7l, aarch64. So I didn’t target a specific device. There is a problem with the cross-compiled wxWidgets (wx-config returning the cross-build paths when used natively) I tried to fix and ran into this issue when cross-building all packages depending on wxWidgets.

[newbluemoon]

Any news on this? :)
I just realized that I could set CXXFLAGS+=" -fsigned-char" for ARM to have the same behaviour as on x86 and compilation finishes successfully. But I don’t know it it’s actually a bug that those negative values are there in the first place.

[baoilleach]

Sorry. I dropped the ball. I'll follow up on all on this in Jan. I think there are some small changes to be made.

[baoilleach]

Note to self: to force at least a warning message on an Intel machine, you can use "-funsigned-char -W -Wall":

/home/noel/Tools/openbabel-baoilleach2/src/formats/pngformat.cpp:221:58: warning: narrowing conversion of '-119' from 'int' to 'const char' inside { } is ill-formed in C++11 [-Wnarrowing]
   const char pngheader[]   = {-119,80,78,71,13,10,26,10,0};

The fix will be to use positive values and "unsigned char" as the type. A patch is on the way. Thanks for reporting.