Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Compiler warning about comparison always false

Reply
Thread Tools

Compiler warning about comparison always false

 
 
Nachy
Guest
Posts: n/a
 
      03-07-2012
Hi, all.

I got an interesting warning message from my compiler, and before I
complain about a bug I want to make sure what's going on.

I have a method disp_char, meant to append a character to a string,
or a period if the character is a control character. The string better
be the right size (it is, that's not the issue).

int disp_char(int c, char *s)
{
char ch = c & 0xFF; // make sure it's a char
if (ch < 0x20 || ch == 0x7F || ch == 0xFF) // special character?
*s = '.';
else
*s = ch;
*++s = '\0';

return 1;
}

On one system I compiled with, gcc gave me a warning:
$ gcc -o ../dump dump.c
dump.c: In function `disp_char':
dump.c:140: warning: comparison is always false due to limited range
of data type

I separated the comparisons, and gcc seems to be complaining about the
last one (ch == 0xFF).

Am I doing something wrong, or is gcc being paranoid?

BTW, on another system, gcc did not complain at all, even with -Wall.
The program seems to work fine, although I can't seem to generate a
test case with an actual file.

Thanks.

-- Nachy
"Thus, the world wide web must never be assigned the acronym WWW." --
RFC 5513
 
Reply With Quote
 
 
 
 
bert
Guest
Posts: n/a
 
      03-07-2012
On Wednesday, March 7, 2012 5:25:57 PM UTC, Nachy wrote:
> Hi, all.
>
> I got an interesting warning message from my compiler, and before I
> complain about a bug I want to make sure what's going on.
>
> I have a method disp_char, meant to append a character to a string,
> or a period if the character is a control character. The string better
> be the right size (it is, that's not the issue).
>
> int disp_char(int c, char *s)
> {
> char ch = c & 0xFF; // make sure it's a char
> if (ch < 0x20 || ch == 0x7F || ch == 0xFF) // special character?
> *s = '.';
> else
> *s = ch;
> *++s = '\0';
>
> return 1;
> }
>
> On one system I compiled with, gcc gave me a warning:
> $ gcc -o ../dump dump.c
> dump.c: In function `disp_char':
> dump.c:140: warning: comparison is always false due to limited range
> of data type
>
> I separated the comparisons, and gcc seems to be complaining about the
> last one (ch == 0xFF).
>
> Am I doing something wrong, or is gcc being paranoid?
>
> BTW, on another system, gcc did not complain at all, even with -Wall.
> The program seems to work fine, although I can't seem to generate a
> test case with an actual file.


You'd expect a warning on a system where the range
of type 'char' was from -128 to +127, and not a
warning on a system where it's from 0 to +255.
Maybe you should be doing the comparisons with
the int result of (c & 0xFF), instead of the
char result, because the constants that you're
comparing with aren't of type 'char'.
--
 
Reply With Quote
 
 
 
 
Nachy
Guest
Posts: n/a
 
      03-07-2012
On Mar 7, 12:32*pm, bert <(E-Mail Removed)> wrote:
> On Wednesday, March 7, 2012 5:25:57 PM UTC, Nachy wrote:
> > I got an interesting warning message from my compiler, and before I
> > complain about a bug I want to make sure what's going on.

>
> > I have a method *disp_char, meant to append a character to a string,
> > or a period if the character is a control character. The string better
> > be the right size (it is, that's not the issue).

>
> > int disp_char(int c, char *s)
> > {
> > * *char ch = c & 0xFF; * * * * * * * * * * * * * * * * * * * * // make sure it's a char
> > * *if (ch < 0x20 || ch == 0x7F || ch == 0xFF) * * * * * // special character?
> > * * * * * **s = '.';
> > * *else
> > * * * * * **s = ch;
> > * **++s = '\0';

>
> > * *return 1;
> > }

>
> > On one system I compiled with, gcc gave me a warning:
> > $ gcc -o ../dump dump.c
> > dump.c: In function `disp_char':
> > dump.c:140: warning: comparison is always false due to limited range
> > of data type

>
> > I separated the comparisons, and gcc seems to be complaining about the
> > last one (ch == 0xFF).

>
> > Am I doing something wrong, or is gcc being paranoid?

>
> > BTW, on another system, gcc did not complain at all, even with -Wall.
> > The program seems to work fine, although I can't seem to generate a
> > test case with an actual file.

>
> You'd expect a warning on a system where the range
> of type 'char' was from -128 to +127, and not a
> warning on a system where it's from 0 to +255.
> Maybe you should be doing the comparisons with
> the int result of (c & 0xFF), instead of the
> char result, because the constants that you're
> comparing with aren't of type 'char'.
> --


Thanks. I've been working with another language recently (figure out
which one), one without explicitly 'signed' and 'unsigned' data types,
where char is always unsigned (and 2 bytes...), so I forgot that
detail. I'll change it to unsigned char to fix.

I'm using an int because that's what fgetc() returns, and I'm using a
char because I don't think *s = ch will work if ch is an int - I'd
rather not deal with implicit casts.

I appreciate the help.

-- Nachy
"Thus, the world wide web must never be assigned the acronym WWW." --
RFC 5513
 
Reply With Quote
 
Philip Lantz
Guest
Posts: n/a
 
      03-07-2012
Nachy wrote:
> On Mar 7, 12:32*pm, bert wrote:
> > On Wednesday, March 7, 2012 5:25:57 PM UTC, Nachy wrote:
> > > I got an interesting warning message from my compiler, and before I
> > > complain about a bug I want to make sure what's going on.

> >
> > > int disp_char(int c, char *s)
> > > {
> > > * *char ch = c & 0xFF; * * * * * * * * * * * * * * * * * * * * // make sure it's a char
> > > * *if (ch < 0x20 || ch == 0x7F || ch == 0xFF) * * * * * // special character?
> > > * * * * * **s = '.';
> > > * *else
> > > * * * * * **s = ch;
> > > * **++s = '\0';

> >
> > > * *return 1;
> > > }

> >

>
> I'm using an int because that's what fgetc() returns, and I'm using a
> char because I don't think *s = ch will work if ch is an int - I'd
> rather not deal with implicit casts.


Declaring ch as either char or unsigned char will not avoid an implicit
conversion when assigning to *s, since ch undergoes promotion to int[1]
first and that value is then converted to the type of *s. So, you might
as well declare ch as an int.

If c is a return value of fgetc, I hope you check for EOF before you
call this function; otherwise, the EOF information will be lost when you
do "c & 0xff". Once you have checked for EOF, you can legitimately pass
the value to this function as an unsigned char, since it's guaranteed to
be in range. Then you could omit ch and the mask operation altogether
and just use c directly.


[1] It could be promoted to unsigned int in some unusual
implementations, but let's not get into that here.
 
Reply With Quote
 
James Kuyper
Guest
Posts: n/a
 
      03-07-2012
On 03/07/2012 12:42 PM, Nachy wrote:
> On Mar 7, 12:32 pm, bert <(E-Mail Removed)> wrote:
>> On Wednesday, March 7, 2012 5:25:57 PM UTC, Nachy wrote:
>>> I got an interesting warning message from my compiler, and before I
>>> complain about a bug I want to make sure what's going on.

>>
>>> I have a method disp_char, meant to append a character to a string,
>>> or a period if the character is a control character. The string better
>>> be the right size (it is, that's not the issue).

>>
>>> int disp_char(int c, char *s)
>>> {
>>> char ch = c & 0xFF; // make sure it's a char
>>> if (ch < 0x20 || ch == 0x7F || ch == 0xFF) // special character?
>>> *s = '.';
>>> else
>>> *s = ch;
>>> *++s = '\0';

>>
>>> return 1;
>>> }

>>
>>> On one system I compiled with, gcc gave me a warning:
>>> $ gcc -o ../dump dump.c
>>> dump.c: In function `disp_char':
>>> dump.c:140: warning: comparison is always false due to limited range
>>> of data type

>>
>>> I separated the comparisons, and gcc seems to be complaining about the
>>> last one (ch == 0xFF).

>>
>>> Am I doing something wrong, or is gcc being paranoid?

>>
>>> BTW, on another system, gcc did not complain at all, even with -Wall.
>>> The program seems to work fine, although I can't seem to generate a
>>> test case with an actual file.

>>
>> You'd expect a warning on a system where the range
>> of type 'char' was from -128 to +127, and not a
>> warning on a system where it's from 0 to +255.
>> Maybe you should be doing the comparisons with
>> the int result of (c & 0xFF), instead of the
>> char result, because the constants that you're
>> comparing with aren't of type 'char'.
>> --

>
> Thanks. I've been working with another language recently (figure out
> which one), one without explicitly 'signed' and 'unsigned' data types,
> where char is always unsigned (and 2 bytes...), so I forgot that
> detail. I'll change it to unsigned char to fix.
>
> I'm using an int because that's what fgetc() returns, and I'm using a
> char because I don't think *s = ch will work if ch is an int - I'd
> rather not deal with implicit casts.


"If the end-of-file indicator for the input stream pointed to by stream
is not set and a next character is present, the fgetc function obtains
that character as an unsigned char converted to an int ..." (7.21.7.1p2).

If CHAR_MAX < 0xFF, which is implied by the warning message you're
complaining about, then if c is a value returned by fgetc(), the
expression (c & 0xFF) can still have a value > CHAR_MAX, in which case
the statement

char ch = c & 0xFF;

attempts to convert to char a value that cannot be represented in a
char. For a signed type, this will either produce an
implementation-defined result, or raise an implementation-defined
signal. The signal is rather unlikely, and the implementation-defined
result stands a very good chance of being exactly what you want -
compiler vendors aren't generally suicidal, and on most systems the
behavior you want is precisely the most natural way of implementing such
conversions. However, it would be better if the validity of your code
wasn't left to chance. You should declare them as "unsigned char ch" and
"unsigned char *s"; then the only problem you need to deal with is EOF.
 
Reply With Quote
 
Fred
Guest
Posts: n/a
 
      03-07-2012
On Mar 7, 11:06*am, James Kuyper <(E-Mail Removed)> wrote:
> On 03/07/2012 12:42 PM, Nachy wrote:
>
>
>
>
>
> > On Mar 7, 12:32 pm, bert <(E-Mail Removed)> wrote:
> >> On Wednesday, March 7, 2012 5:25:57 PM UTC, Nachy wrote:
> >>> I got an interesting warning message from my compiler, and before I
> >>> complain about a bug I want to make sure what's going on.

>
> >>> I have a method *disp_char, meant to append a character to a string,
> >>> or a period if the character is a control character. The string better
> >>> be the right size (it is, that's not the issue).

>
> >>> int disp_char(int c, char *s)
> >>> {
> >>> * *char ch = c & 0xFF; * * * * * * * * * * * * * * * * * * * * // make sure it's a char
> >>> * *if (ch < 0x20 || ch == 0x7F || ch == 0xFF) * * ** * // special character?
> >>> * * * * * **s = '.';
> >>> * *else
> >>> * * * * * **s = ch;
> >>> * **++s = '\0';

>
> >>> * *return 1;
> >>> }

>
> >>> On one system I compiled with, gcc gave me a warning:
> >>> $ gcc -o ../dump dump.c
> >>> dump.c: In function `disp_char':
> >>> dump.c:140: warning: comparison is always false due to limited range
> >>> of data type

>
> >>> I separated the comparisons, and gcc seems to be complaining about the
> >>> last one (ch == 0xFF).

>
> >>> Am I doing something wrong, or is gcc being paranoid?

>
> >>> BTW, on another system, gcc did not complain at all, even with -Wall.
> >>> The program seems to work fine, although I can't seem to generate a
> >>> test case with an actual file.

>
> >> You'd expect a warning on a system where the range
> >> of type 'char' was from -128 to +127, and not a
> >> warning on a system where it's from 0 to +255.
> >> Maybe you should be doing the comparisons with
> >> the int result of (c & 0xFF), instead of the
> >> char result, because the constants that you're
> >> comparing with aren't of type 'char'.
> >> --

>
> > Thanks. *I've been working with another language recently (figure out
> > which one), one without explicitly 'signed' and 'unsigned' data types,
> > where char is always unsigned (and 2 bytes...), so I forgot that
> > detail. *I'll change it to unsigned char to fix.

>
> > I'm using an int because that's what fgetc() returns, and I'm using a
> > char because I don't think *s = ch will work if ch is an int - I'd
> > rather not deal with implicit casts.

>
> "If the end-of-file indicator for the input stream pointed to by stream
> is not set and a next character is present, the fgetc function obtains
> that character as an unsigned char converted to an int ..." (7.21.7.1p2).
>
> If CHAR_MAX < 0xFF, which is implied by the warning message you're
> complaining about, then if c is a value returned by fgetc(), the
> expression (c & 0xFF) can still have a value > CHAR_MAX, in which case
> the statement
>
> * * * * char ch = c & 0xFF;
>
> attempts to convert to char a value that cannot be represented in a
> char. For a signed type, this will either produce an
> implementation-defined result, or raise an implementation-defined
> signal. The signal is rather unlikely, and the implementation-defined
> result stands a very good chance of being exactly what you want -
> compiler vendors aren't generally suicidal, and on most systems the
> behavior you want is precisely the most natural way of implementing such
> conversions. However, it would be better if the validity of your code
> wasn't left to chance. You should declare them as "unsigned char ch" and
> "unsigned char *s"; then the only problem you need to deal with is EOF.- Hide quoted text -
>
> - Show quoted text -


Also, comparing to 0x20, 0x7F, and 0xFF is probably not the best way
to do things.
First of all, yu are depending on a specific character set.
Second, it is somewhat cryptic - not everyone remembers what those
values
really mean. Better to use isprint() or some other ctype methods,
which
are much more descriptive of what you are trying to accomplish.
--
Fred K
 
Reply With Quote
 
Markus Wichmann
Guest
Posts: n/a
 
      03-07-2012
On 07.03.2012 18:25, Nachy wrote:
> Hi, all.
>
> I got an interesting warning message from my compiler, and before I
> complain about a bug I want to make sure what's going on.
>
> I have a method disp_char, meant to append a character to a string,
> or a period if the character is a control character. The string better
> be the right size (it is, that's not the issue).
>
> int disp_char(int c, char *s)
> {
> char ch = c & 0xFF; // make sure it's a char
> if (ch < 0x20 || ch == 0x7F || ch == 0xFF) // special character?
> *s = '.';
> else
> *s = ch;
> *++s = '\0';
>
> return 1;
> }
>
> On one system I compiled with, gcc gave me a warning:
> $ gcc -o ../dump dump.c
> dump.c: In function `disp_char':
> dump.c:140: warning: comparison is always false due to limited range
> of data type
>


If you want ch to be unsigned, why don't you tell the compiler? But why
go such lengths at all? c can have those values, too, and more. If c is
the result from a call to fgetc, you should check for an error or
end-of-file by comparing it to EOF.

The result of fgetc() is defined to be either a character or EOF. Also
you can make this function 100% portable by including <ctype.h> and
replacing the condition of the if with !isprint(c).

> I separated the comparisons, and gcc seems to be complaining about the
> last one (ch == 0xFF).
>
> Am I doing something wrong, or is gcc being paranoid?
>


char may be signed or unsigned. Usually an integer type without special
qualifier is signed, but char is special. Basically, int is defined to
be the same as signed int, but char is defined to be the same as signed
char _or_ unsigned char. Programmers usually cope by treating char,
signed char and unsigned char three distinct types and try not to make
use of arithmetic on char other than what is defined to work. For
instance, the C standard defines the digit characters to be in order,
but not the letters. That means

r = 0;
while (isdigit(*s))
r = 10 * r + *s++ - '0';

is a portable way to parse a decimal number, but

r = 0;
while (isxdigit(*s))
if (isdigit(*s)
r = 16 * r + *s++ - '0';
else if (isupper(*s))
r = 16 * r + *s++ - 'A' + 10;
else
r = 16 * r + *s++ - 'a' + 10;

is not a portable way to parse a hexadecimal number. If you want to make
it 100% portable, you'd have to do something like:

char digits[] = "0123456789ABCDEF";
unsigned int r = 0;
char c, *p;
for (c = *s; c; c = *++s)
{
if (islower(c)) c = toupper(c);
p = strchr(digits, c);
if (p) r = 16 * r + (p - digits);
else break;
}

> BTW, on another system, gcc did not complain at all, even with -Wall.
> The program seems to work fine, although I can't seem to generate a
> test case with an actual file.
>


Try compiling with --signed-char on those systems.

> Thanks.
>
> -- Nachy


Please insert a newline after the dash-dash-space, then it gets treated
as signature and is cut off automatically when answering.

Ciao,
Markus
 
Reply With Quote
 
Edward A. Falk
Guest
Posts: n/a
 
      03-07-2012
In article <(E-Mail Removed)>,
Nachy <(E-Mail Removed)> wrote:

>I separated the comparisons, and gcc seems to be complaining about the
>last one (ch == 0xFF).


Your compiler uses signed characters, which can't have a value greater
than 0x7F. 0xFF is an integer, so the comparison will first be promoting
ch to an integer. The compiler knows that ch will never have the value
0xFF and so the comparison will always be false, which is probably not
what you had in mind, so the compiler is doing you a big favor and
warning you that this code will not do what you want.

Fix:

ch == (char)0xFF

--
-Ed Falk, http://www.velocityreviews.com/forums/(E-Mail Removed)
http://thespamdiaries.blogspot.com/
 
Reply With Quote
 
Keith Thompson
Guest
Posts: n/a
 
      03-07-2012
(E-Mail Removed) (Edward A. Falk) writes:
> In article <(E-Mail Removed)>,
> Nachy <(E-Mail Removed)> wrote:
>
>>I separated the comparisons, and gcc seems to be complaining about the
>>last one (ch == 0xFF).

>
> Your compiler uses signed characters, which can't have a value greater
> than 0x7F. 0xFF is an integer, so the comparison will first be promoting
> ch to an integer.


More precisely, 0xFF is an int. The word "integer" covers a variety of
types, including both int and char.

> The compiler knows that ch will never have the value
> 0xFF and so the comparison will always be false, which is probably not
> what you had in mind, so the compiler is doing you a big favor and
> warning you that this code will not do what you want.


Maybe. But if the code is intended to be portable to systems where
plain char is either signed or unsigned, the fact that the comparison is
always true on some systems doesn't mean it doesn't make sense on
others.

> Fix:
>
> ch == (char)0xFF


I'd say that's more of a workaround. The result of converting an
out-of-range value to a signed type is implementation-defined (it can
even raise a signal). If plain char is signed and CHAR_BIT==8, then
(char)0xFF is *probably* -1, but that's not guaranteed.

A better solution, as other responses have said, is probably to change
the type of ch, most likely to int.

--
Keith Thompson (The_Other_Keith) (E-Mail Removed) <http://www.ghoti.net/~kst>
Will write code for food.
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
 
Reply With Quote
 
Tim Rentsch
Guest
Posts: n/a
 
      03-13-2012
Philip Lantz <(E-Mail Removed)> writes:

> Nachy wrote:
>
>> On Mar 7, 12:32 pm, bert wrote:
>> > On Wednesday, March 7, 2012 5:25:57 PM UTC, Nachy wrote:
>> > > I got an interesting warning message from my compiler, and before I
>> > > complain about a bug I want to make sure what's going on.
>> >
>> > > int disp_char(int c, char *s)
>> > > {
>> > > char ch = c & 0xFF; // make sure it's a char
>> > > if (ch < 0x20 || ch == 0x7F || ch == 0xFF) // special character?
>> > > *s = '.';
>> > > else
>> > > *s = ch;
>> > > *++s = '\0';
>> >
>> > > return 1;
>> > > }
>> >

>>
>> I'm using an int because that's what fgetc() returns, and I'm using a
>> char because I don't think *s = ch will work if ch is an int - I'd
>> rather not deal with implicit casts.

>
> Declaring ch as either char or unsigned char will not avoid an implicit
> conversion when assigning to *s, since ch undergoes promotion to int[1]
> first and that value is then converted to the type of *s. [snip]


It's true there is a conversion for the assignment '*s = ch', to
the type of the LHS (always true, for assignment), but there is
no promotion to int or any other type - just a single conversion,
to the type of the LHS, from the type of the RHS.
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
warning: comparison is always true due to limited range of data type phil-news-nospam@ipal.net C Programming 6 10-26-2009 11:50 PM
comparison is always false John Ratliff C++ 8 02-04-2007 10:13 PM
debug="false" in web.config and <%@ debug="true" ...%> in aspx file => true or false? André ASP .Net 3 08-28-2006 12:02 PM
False positive, false intrusion, false alarm Nick Computer Security 3 04-26-2006 07:40 PM
comparison is always false due to limited range of data type Dave C Programming 2 02-26-2005 02:51 AM



Advertisments