MC9S08DZ60 and CW6.22

[aergin]

I am having trouble getting a macro to work in my application and I am wondering if anyone knows a way to fix this with compiler settings.  This is part of trunk code that spans over several compilers so chaning the code would make things difficult.

Here is the macro:

 #define TimeOver(soll,ist) ((UINT16) ((ist) - (soll)) < 0x8000)

  And here is one instance of where it is used:

  if(TimeOver (FUTURE_TIME, CURRENT_Timer))
  {
    ...
  }

Basically, it is a timeout mechanism.  The problem is the condition always comes out true even when it is not.  I know this macro works on at least 5 or 6 other compilers, but I can't get it to work correctly in this case.

For debugging purposes, I added in a temp variable so I could see what was going on with the TimeOver macro and the system started working.

  temp =  TimeOver (FUTURE_TIME, CURRENT_Timer);

  if(temp)
  {
    ...
  } 

Does anyone have an idea of why this could be going on?  Thanks in advance.

[Lundin · Accepted solution]

There are many problems with that code. The main one is that it's a macro... something you probably won't like to hear. Macros only lead to bugs and more bugs and debugging problems and obscure compiler/linker warnings/erros and there is never a need to use them, as all modern compilers support inlining in some way.

Aside from that main problem, here are the portability issues:

First you assume that the compiler is using a certain type of signed integer formatting. The C standard allows both one's compliment and two's compliment for signed integers. Practically, every known CPU and C compiler is using two's compliment (Freescale CPUs and Codewarrior does this), but you can't assume it if you want your code to be portable.

(ist) - (soll)
This results in a variable of type "int", which could be signed or unsigned, we don't know which, as this is implementation-specific behavior. It could also possibly be 32 bits.

(UINT16) ((ist) - (soll))
Ok now we have eliminated the risk of 32 bits. The type is most definitely UINT16 at this point.
So we have the expression

UINT16 [something] < 0x8000

If UINT16 is defined as unsigned short, then the "integer promotions" in C apply. In that case, the type is once again "int", which may be signed or unsigned, 16 or 32 bits. Lets assume it is defined as unsigned int for the sake of the discussion.

Then the result of the operation still depends on what implementation-specific type "int" is, because 0x8000 is an "int", which could be signed or unsigned, 16 or 32 bits.

If "int" is signed, the "usual arithmetic conversions" will convert 0x8000 to a signed integer. The result will be, assuming 16 bit integers, (ist - soll) < -32768.

If "int" is unsigned, the "usual arithmetic conversions" will convert 0x8000 to an unsigned integer. The result will be (ist-soll) < 32768.


The solution to the problem would be something like this:

BOOL TimeOver (uint16_t soll, uint16_t ist)
{
  uint16_t result = ist - soll;

  return result < 0x8000U;
}

Message Edited by Lundin on 2009-12-15 03:03 PM

[aergin]

I appreciate the response.  I agree, I don't like macros either.  I guess the main problem I have is that I can make it work by rewritting but I have to get it working without changing the code.  I was hoping that messing around with compiler settings would help, but I could not find anything.