Help: warning: comparison is always false due to .....

Go To Last Post
14 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Guyzz i'm still struggeling this USBASP a bit ....

Im now compiling fine , and even linking ok (See thread link below) :oops:

I get this warning

usbdrv/usbdrv.c:386: warning: comparison is always false due to limited range of data type

when making one of the modules , (i don't with the original makefile) , but that could be that the WinAVR makefile uses more strict checkking

Full compile output here

Compiling: usbdrv/usbdrv.c
avr-gcc -c -mmcu=atmega8 -I. -gdwarf-2 -DF_CPU=12000000UL -DXTAL=12.0000e6 -DDEBUG_LEVEL=0 -DPLATFORM=0  -Os -funsigned-char -funsigned-bitfields -fpack-struct -fshort-enums -Wall -Wstrict-prototypes -Wa,-adhlns=usbdrv/usbdrv.lst -Iusbdrv -std=gnu99 -MD -MP -MF .dep/usbdrv.o.d usbdrv/usbdrv.c -o usbdrv/usbdrv.o 
usbdrv/usbdrv.c: In function `usbPoll':
usbdrv/usbdrv.c:386: warning: comparison is always false due to limited range of data type

Here is "Snip of Var definitions" , and the routine that enerates the warning.

/* ------------------------------------------------------------------------- */

/* raw USB registers / interface to assembler code: */
/* usbRxBuf MUST be in 1 byte addressable range (because usbInputBuf is only 1 byte) */
static char	usbRxBuf[2][USB_BUFSIZE];/* raw RX buffer: PID, 8 bytes data, 2 bytes CRC */
uchar		usbDeviceId;		/* assigned during enumeration, defaults to 0 */
uchar		usbInputBuf;		/* ptr to raw buffer used for receiving */
uchar		usbAppBuf;			/* ptr to raw buffer passed to app for processing */
volatile char usbRxLen;			/* = 0; number of bytes in usbAppBuf; 0 means free */
uchar		usbCurrentTok;		/* last token received */
uchar		usbRxToken;			/* token for data we received */
uchar		usbMsgLen = 0xff;	/* remaining number of bytes, no msg to send if -1 (see usbMsgPtr) */
volatile char usbTxLen = -1;	/* number of bytes to transmit with next IN token */
uchar		usbTxBuf[USB_BUFSIZE];/* data to transmit with next IN, free if usbTxLen == -1 */
#if USB_CFG_HAVE_INTRIN_ENDPOINT
/* uchar		usbRxEndp;		endpoint which was addressed (1 bit in MSB) [not impl] */
volatile char usbTxLen1 = -1;	/* TX count for endpoint 1 */
uchar		usbTxBuf1[USB_BUFSIZE];/* TX data for endpoint 1 */
#endif
uchar		usbAckBuf[1] = {USBPID_ACK};	/* transmit buffer for ack tokens */
uchar		usbNakBuf[1] = {USBPID_NAK};	/* transmit buffer for nak tokens */

/* USB status registers / not shared with asm code */
uchar			*usbMsgPtr;		/* data to transmit next -- ROM or RAM address */
static uchar	usbMsgFlags;	/* flag values see below */
static uchar	usbNewDeviceId;	/* = 0; device ID which should be set after status phase */
static uchar	usbIsReset;		/* = 0; USB bus is in reset phase */



/* ------------------------------------------------------------------------- */

void	usbPoll(void)
{
  uchar	len;
 
	if((len = usbRxLen) > 0){
/* We could check CRC16 here -- but ACK has already been sent anyway. If you
 * need data integrity checks with this driver, check the CRC in your app
 * code and report errors back to the host. Since the ACK was already sent,
 * retries must be handled on application level.
 * unsigned crc = usbCrc16((uchar *)(unsigned)(usbAppBuf + 1), usbRxLen - 3);
 */
		len -= 3;	/* remove PID and CRC */
		if(len < 128){
			usbProcessRx((uchar *)(unsigned)(usbAppBuf + 1), len);
		}
		usbRxLen = 0;	/* mark rx buffer as available */
	}

***** Warning points at line below *********************

**--->	if(usbTxLen < 0){	/* TX system is idle */
		if(usbMsgLen != 0xff){
			usbBuildTxBlock();
		}else if(usbNewDeviceId){
			usbDeviceId = usbNewDeviceId;
			DBG1(1, &usbNewDeviceId, 1);
			usbNewDeviceId = 0;
		}
	}
	if(isNotSE0()){	/* SE0 state */
		usbIsReset = 0;
	}else{
		/* check whether SE0 lasts for more than 2.5us (3.75 bit times) */
		if(!usbIsReset){
			uchar i;
			for(i=100;i;i--){
				if(isNotSE0())
					goto notUsbReset;
			}
			usbIsReset = 1;
			usbDeviceId = 0;
			usbNewDeviceId = 0;
			DBG1(0xff, 0, 0);
notUsbReset:;
		}
	}
}

Program download & Info is here
https://www.avrfreaks.net/index.p...

I have tried to make "uchar len" to a "char len" , and tried to change to
if(usbTxLen < (char) 0){ /* TX system is idle */

But GCC still complains :-(

What have i missed ???

/Bingo

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

"-funsigned-char" in your makefile tells GCC that all char variables are unsigned unless you explicitly state that it should be signed. "char VAR" is the same as "unsigned char VAR" unless you explicitly type "signed char VAR" and so the unsigned char test against a less-than-zero number is always false.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Update:

If i change if(usbTxLen < 0){ , to if(usbTxLen == 0){

The error goes away , but it's still there if i change to if((char) usbTxLen < (char)0) ???

With this declaration , it should be legal to have a negative usbTxLen
volatile char usbTxLen = -1;

So what is wrong ??

/Bingo

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

abcminiuser wrote:
"-funsigned-char" in your makefile tells GCC that all char variables are unsigned unless you explicitly state that it should be signed. "char VAR" is the same as "unsigned char VAR" unless you explicitly type "signed char VAR" and so the unsigned char test against a less-than-zero number is always false.

- Dean :twisted:

Ahhh :-)

That was one thing i didn't look into (Thnx Dean)

Sounds plausible

I'll investigate further

Who the F..... did default to that in the makefile :cry: :cry:

/Bingo

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You can use the header file and the types it defines (int8_t in this case).
Intro here:
http://www.embedded.com/shared/printableArticle.jhtml?articleID=17300092
/Lars

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

@Dean

Spot on ....

removing -funsigned-char , did it :-)

/Bingo

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If you force "char" to be unsigned by the compiler option
-funsigned-char, it cannot become negative, so the comparision for
usbTxLen < 0 will be always true.

Of course, the original program isn't written in a good style, as it
relies on the signedness of the default "char" which is not guaranteed
to be portable. It should declare usbTxLen as "signed char", or even
better include , and use "int8_t".

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

@all

Thats why i have not been hit bu this one before :-)

I always use uint8_t & int8_t , and then the -funsigned-char is ok , as int8_t expands to signed char

******** Snip from stdint.h *************

/** \ingroup avr_stdint
    8-bit signed type. */

typedef signed char int8_t;

/** \ingroup avr_stdint
    8-bit unsigned type. */

typedef unsigned char uint8_t;

Thanx guyzz :-)

Ohh btw: I normally include , and had a look at that one
it just includes .

Witch one would be the correct one to use ???
Is part of the "Standard" and inttypes , just there for compatibility ??

/Bingo

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

is a superset of . For just the type
definitions, the latter is OK. It's only that some early
C99-half-compliant versions of GCC came with only
(perhaps based on an early draft of C99, I
don't know).

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This is suspicious:

if((len = usbRxLen) > 0){

Even if it works I wouldn't do it. What it the point in making the code harder to read than it has to be?

len = usbRxLen;
if(len > 0){
....

Ken

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It's common C coding style. Just
like foo += 3; is, or PORTB |= (1<<x);

There are situations where the workaround would
look a lot more complicated indeed, as for e.g.

while ((len = usbRxLen) > 0) {
...
}

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
It's common C coding style.

I'll take your word for it because you seem to know what you are tlaking about but I've never seen it. And, I'm sure why you would want to write it that way.

Am I missing something? len is assigned the value of usbRxLen. Then Len is compared to zero. True?

It just seems like the assignment doesn't have anything to do with the if()

Ken

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

DUH! At lunch I realized what you were saying. It makes more sense if it were a function in a while loop.

while((c = getchar()) != EOF)
{
...
}

Is the classical example. Of course.
Yes, I have seen and used this. I guess it's been a while though.

Ken

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Bingo600 wrote:

Who the F..... did default to that in the makefile :cry: :cry:
/Bingo

I did. :)

For the same reasons that Joerg mentioned about portability. I have a seen a plain char have different signedness on different compilers. These days you will see a compiler switch on a lot of commercial cross-compilers (and native compilers too) that forces char to be a particular default signedness. IIRC, IAR has this switch too. A good 99% of the time, a developer wants char to be an unsigned char because it is used to hold "character" data, rather than use it as an 8 bit integer. So that is why I put the switch in the Makefile to default it to unsigned.

As other people have said, that is why is so useful because you can easily specify an integer, the size and the signedness, and you visually see that in the name of the type.

HTH
Eric