Allegro.cc - Online Community

Allegro.cc Forums » Programming Questions » Clearing errors in GCC

This thread is locked; no one can reply to it. rss feed Print
Clearing errors in GCC
nshade
Member #4,372
February 2004

So I'm porting some legacy DOS code to Windows and I have the game compiling under Visual Studio with no errors or warnings. Over the weekend I ported the game to Linux, and I'm in the process of cleaning up the warnings that GCC is giving me. (It appears that it's a little more picky over warnings)

I have GCC down to 10 warnings and I have two warning types left that I'm stuck on and don't quite know what the compiler is asking of me.

The first warning:
Warning: Operation on 'temp2' may be undefined (-Wsequence-point)
In this case I made sure that temp2 was initialized to 0, but I think it's asking for something else as it still is giving this warning. Here's the code it's wagging it's finger about.

/* Compute need factor */
temp = EXP / 13;
while (supply[temp] == -1) temp--;
pref[temp] = 8;
temp2 = temp - 1;
while (temp2 >= 0) pref[temp2--] = 8 - (temp - temp2);    //<--- Warning here
temp2 = temp + 1;
while (temp2 <= 7) pref[temp2++] = 8 - (temp2 - temp);  //<---Warning here
preftot = 0;
temp2 = 0;

The other warning class I'm not understanding is this
Warning: cast from pointer to integer of different size (-Wpointer-to-int-cast)
here is the example code

#SelectExpand
1void IceptStar(FwgPtr f, SolPtr p) 2{ 3 int angle; 4 5 f->pos.navlock = 1; 6 f->pos.locktype = 6; 7 f->pos.lockto = NULL; 8 if ((int)p == 1) { //<----Warning here 9 f->pos.lockx = Game->solar.x1; 10 f->pos.locky = Game->solar.y1; 11 f->pos.lockr = Game->solar.starradius; 12 } 13 else { 14 f->pos.lockx = Game->solar.x2; 15 f->pos.locky = Game->solar.y2; 16 f->pos.lockr = Game->solar.star2radius; 17 } 18//...etc 19}

I can kinda see why this is acting strange. p is a pointer to a struct called SolarStru, whcih holds the solar system data. I have no idea what that cast is doing. How does casting a pointer to an int give you an intelligible answer to work off of? I can see why the compiler is throwing the warning. What would be the original motivation to do this and how would I approach making it better?

Audric
Member #907
January 2001

The second one :
If p is actually used as a pointer somewhere in the function (p->member), then this is a case where the code sometimes uses a "magic value" 1 for a specific behavior, the call looking like IceptStar(someaddress, 1).
The numeric value 1 would never happen as a real pointer, because it is actually the memory address of the second byte of the NULL pointer. This is never an address you can get for a piece of stack, heap or malloc'ed data.

The warning is because SolPtr is a pointer of one size, while "int" has a different size. For example, you may be compiling on a 64bit architecture so pointers are 8 bytes long, while int is still 4 bytes long.
The consequence is that the cast only gets half of the address to interpret as number, and this is suspicious. I'm not 100% sure, but depending on endianness, you may get "p modulo 2^32", or "p divided by 2^32". In the latter case, passing "1" as an argument woulf not enter the condition if ((int)p == 1), and this is probably not what the coder intended.
-> cast to an uint64_t if you're on 64bit system
(edit: fixed type name. uint64_t should be unambiguous and available everywhere)

Peter Hull
Member #1,136
March 2001

First one:
Expressions within a 'sequence' are not necessarily executed right-to-left, so, since you have temp2-- on the LHS, the RHS could be using the value of temp2 either before or after the decrement. See https://stackoverflow.com/questions/40677318/wsequence-point-warning-what-does-it-mean-and-does-it-violate-ansi-c-standard
Just move the decrement to the next statement. (though check this does what you actually want!)

while (temp2 >= 0) {
    pref[temp2] = 8 - (temp - temp2);    
    temp2--;
}

Second one:
To add to what Audric said, if you want an integer that's definitely big enough to hold a pointer, regardless of whether you're on 32 or 64 bit, use intptr_t (or uintptr_t)
See https://wiki.sei.cmu.edu/confluence/display/c/INT36-C.+Converting+a+pointer+to+integer+or+integer+to+pointer

Audric
Member #907
January 2001

About 1, it's not very intuitive because there are specific rules about operator precedence, but it's not a strict order of evaluation (associativity often gets mistaken for order of evaluation as well). The compiler/CPU can "reorder" computations if it's more efficient (or run the in parallel), as long as the rules are respected. For example in exp1 = exp2, both expressions are necessary before the assignment can be performed, but nothing forces exp2 to be evaluated entirely before the evaluation of exp1 begins.

The issues are always when some terms have side effects : increment/decrement, assignment itself, and function evaluation.

torhu
Member #2,727
September 2002
avatar

Using a for statement makes it clearer how the loop is controlled.

for (temp2 = temp - 1; temp2 >= 0; temp2--) {
    pref[temp2] = 8 - (temp - temp2);
}

Go to: