search

aladur / flexemu

An MC6809 emulator running FLEX operating system
GNU General Public License v2.0
16 stars 4 forks source link

char data type comparisons to -1 #4

Closed labomb closed 5 years ago

labomb commented 5 years ago

Several 'comparison is always xxxxx due to limited range of data type' warnings are generated when building on the Raspberry Pi. They all appear to be related to the 'filename' member of the s_dir_entry struct. Each warning is with a comparison of the first character of filename to -1... since filename[0] is type char, it shouldn't ever be equal to -1, and the comparison will always be true or false depending on != or == respectively.

The warnings are at ffilecnt.cpp, iffilcnt.cpp, and in ndircnt.cpp here, here, and here.

They don't appear to have any adverse effect on the operation of flexemu though. Curiously, regardless of which combination of compile flags I've tried, I can't get the warnings on ubuntu 16.04 or 18.04... they only show up when building on the Pi.

These warnings and this other issue aside, the good news is that flexemu appears to run just fine (in terminal mode anyway... haven't tested gui mode yet) on the Pi. Very cool!

aladur commented 5 years ago

According to the C/C++ standard char may be defined as signed or unsigned. It seems that on a x86/x86_64 linux gcc it is typically defined as signed char. On gcc arm it typically seems to be unsigned. Thats the reason why only gcc for Pi complains. I am working on a solution which fits for all and does not make too much noise ...

labomb commented 5 years ago

Yes, I did a bit more research last week and did determine that was the issue (most of my work has been on the x86/64 platform... never knew the char default could vary on other platforms!).

I initially attempted to change filename to signed char, thinking there was just a few string functions here and there that I could do some simple type casting with.... but that certainly wasn't the case :). Also tried using compile flag -fsigned-char for kicks, and while the original warnings went away it also introduced some other issues (and it's a bad approach from a portability perspective anyway).

aladur commented 5 years ago

Issue solved with commit b2c5ed9eef9e783d0976f50644472d1bd11c793b.

labomb commented 5 years ago

That took care of it... nice approach! Thanks much...