BackupGGCode / grafx2

A pixelart-oriented painting program
0 stars 2 forks source link

fix 64bit issue in loadsave.c #313

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
This patch fixes;

loadsave.c: In function ‘Rotate_safety_backups’:
loadsave.c:1347: warning: format ‘%6.6d’ expects type ‘int’, but 
argument 
5 has type ‘long int’
loadsave.c:1357: warning: format ‘%6.6d’ expects type ‘int’, but 
argument 
4 has type ‘long int’

The ideal solution is to make "Main_safety_backup_prefix" a known type 
(ie: Uint32 or Uint64) instead of long. ouch looked in global.h all those 
long/short/dword/word need upadting to int32_t/int16_t etc

Original issue reported on code.google.com by yakumo9...@gmail.com on 6 Feb 2010 at 3:41

Attachments:

GoogleCodeExporter commented 9 years ago
the problem is we want to have a very high level of compatibility, including 
some 
systems that are still somewhat in C89...
int16_t and int32_t are C99, so we can't really make use of them in a direct 
way.

Original comment by pulkoma...@gmail.com on 7 Feb 2010 at 11:09

GoogleCodeExporter commented 9 years ago

Original comment by pulkoma...@gmail.com on 7 Feb 2010 at 11:43

GoogleCodeExporter commented 9 years ago
I understand that there are still a lot of c89 era level compilers around (heck 
even
vs2003 didnt have it! I've run into it myself several times).

There is a BSD license portable header for stdint.h that can be used on these 
systems.

http://www.azillionmonkeys.com/qed/pstdint.h

It makes life so much easier.. that or just use the SDL built in types Uint32, 
Uint16
etc. But I'd seriously urge thinking about replaceing the 
long/short/word/dword. I
dont mind doing patches :)

Original comment by yakumo9...@gmail.com on 7 Feb 2010 at 12:45

GoogleCodeExporter commented 9 years ago
Then again, you may have noticed that we don't have a full-featured autoconf 
filesystem but just a simple makefile, so testing for stuff sometimes add more 
complexity than working around it.

Original comment by pulkoma...@gmail.com on 7 Feb 2010 at 12:49

GoogleCodeExporter commented 9 years ago
So use the SDL types Int32, Int16, Uint32, Uint16 etc. If you look in the SDL 
headers
they just typedef cast uint32_t to Uint32 anyway so they are somehow hacking 
around
it missing (maybe on ports on these old systems the porters just predefine it to
native types).

Would you be interested in patches for using SDL types? Is there a dev mailing 
list?
I posted a message in the discussion forum, may be better than posting here in 
the
bug report :)

Original comment by yakumo9...@gmail.com on 7 Feb 2010 at 1:04

GoogleCodeExporter commented 9 years ago
The bug report is fine by me for recording discussion.
We already thought a bit about the types, in issue 140. (these links are also 
handy)
I have to say I have forgotten a bit about that. I kinda remember I use "long" 
to
ensure the type could hold numbers >32767, even on a 16bit compiler. I've used 
"int"
generally in the stack, when I knew the value was <32767 (such as all 
coordinates),
so the compiler could choose the most efficient type for platform, without 
breaking
on 16bit compiler.
Maybe both were bad ideas, because I don't remember if Grafx2 is really 
compilable on
a 16bit compiler in the first place.

I'm a bit too used to bytes to enjoy changing them (But then, I was also 
reluctant to
convert the code to English, and I did... I'm mean, this is my current opinion, 
not a
decision). The concerns should be in descending priority:
1) Don't break other platforms (especially those of active maintainers :)
2) Make grafx2 compile on 64bit compiler without compiler errors
3) Make the 64bit build run without bug
4) Make the 64bit compiler report no warnings

Original comment by yrizoud on 7 Feb 2010 at 4:54

GoogleCodeExporter commented 9 years ago
GrafX2 never worked on any 16bit platform. Even in the dos days it used a dos 
extender. I'd say no one use 16bit CPUs anymore, anyway. We may want a 68000 
build 
for amiga but that would be too slow.

As for 64bit, note I'm using that everyday and grafx2 runs perfectly fine, the 
fixes 
were only minor things to remove warnings. So we already pass steps 1 to 3, and 
now 
we pass 4 again.

I'd like to keep byte/(d/q)word too, but using short/int/long is probably 
wrong. 
What other names could we use ?

Original comment by pulkoma...@gmail.com on 7 Feb 2010 at 5:04

GoogleCodeExporter commented 9 years ago
Well, despite running on 32bit CPU, Borland TurboC++ on DOS had 16bit "int" (an 
int
was implicitely short). I don't remember if Watcom C was like this or not. This 
is
why I don't know how to call the issue.. 32bit platform, 32bit compiler...

I looked a little. Short is always 16bit, so it seems to be reliable signed 
version
of 'word'. It's the most used type (except the universal byte and word), so I
hesitate to change it. If I have to stop using the names "int" and "long" 
however, I
can change my habits, no problem.

In any case, don't hesitate to report compilation warnings as an issue. 
Sometimes
it's me being sloppy, or a compiler difference which makes some platforms detect
different things.

Original comment by yrizoud on 7 Feb 2010 at 6:18

GoogleCodeExporter commented 9 years ago
short has always been 16bit, int can change between 16/32bit archs. long also.

in dos, int = 16bit, long = 32bit. on 32bit, int = 32bit, long = 32bit. on 
64bit its different again because you have the whole ILP64 & LLP64 thing.

look here for a nice table;

http://www.unix.org/version2/whatsnew/lp64_wp.html

since were using 32 + 64bit arch every day, long is the only one that can 
really impact right now. But as pointed out, everything is working ok without 
changes so I'll just keep my eye on 
compiler warnings and such

Original comment by yakumo9...@gmail.com on 7 Feb 2010 at 6:50

GoogleCodeExporter commented 9 years ago
DOS is a bit more complex than that actually...

There is the plain DOS which is a 16bit OS not using the 32bit capabilities at 
all 
(basically it will run on a 286). To use the full power of the CPU, you have to 
use 
a 32bit compiler and a "dos extender". Watcom provides both a 16bit and a 32bit 
compiler, you can switch (or even mix them in a project) to make whatever you 
need. 
GrafX2 used Eclipse WEOS dos extender and ran in a 32bit world. The DOS 
extender 
handles all the communication with the core DOS system when you want to do a 
system 
operation (calling an interrupt, usually).

As for the size of various types, the C89 standard says :
sizeof(char) == 1
char is big enough to hold a character
int : native type of the CPU
sizeof(short) <= sizeof(int) <= sizeof(long)

That's about it. If you run a pure ASCII system, you may decide to have 7bit in 
a 
char...
However, I think we're fine sticking to 32 and 64bit for now. But still note 
that 
some 64bit platform (besides x86) may decide to have int 64bit, short 32, short 
short 16, for example. Or use the more modern C99 int32_t and friends which are 
clear regarding the size and solve the problem.

Original comment by pulkoma...@gmail.com on 7 Feb 2010 at 7:10

GoogleCodeExporter commented 9 years ago
c99 iterates as well sizeof(char) = 1 as well.  Ive written dos extenders and 
commercial apps with watcom 10.6 
using its 16 + 32bit and its novel netware compilers. I was most familiar with 
trans pmode and adam seychells 
dos32.

the only funky dos extenders I encountered were the old pharlap 286 extended 
modes and dealing with the 386 
voodoo modes in demos

It would be interesting to see grafx2 working on a 64bit of big endian but I 
dont have any 64bit sparc/ppc 
machines anymore.

Original comment by yakumo9...@gmail.com on 7 Feb 2010 at 7:19