Velocity Reviews

Velocity Reviews (http://www.velocityreviews.com/forums/index.php)
-   C Programming (http://www.velocityreviews.com/forums/f42-c-programming.html)
-   -   Compiler warning about comparison always false (http://www.velocityreviews.com/forums/t869119-compiler-warning-about-comparison-always-false.html)

Nachy 03-07-2012 05:25 PM

Compiler warning about comparison always false
 
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

bert 03-07-2012 05:32 PM

Re: Compiler warning about comparison always false
 
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'.
--

Nachy 03-07-2012 05:42 PM

Re: Compiler warning about comparison always false
 
On Mar 7, 12:32*pm, bert <bert.hutchi...@btinternet.com> 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

Philip Lantz 03-07-2012 06:17 PM

Re: Compiler warning about comparison always false
 
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.

James Kuyper 03-07-2012 07:06 PM

Re: Compiler warning about comparison always false
 
On 03/07/2012 12:42 PM, Nachy wrote:
> On Mar 7, 12:32 pm, bert <bert.hutchi...@btinternet.com> 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.

Fred 03-07-2012 07:11 PM

Re: Compiler warning about comparison always false
 
On Mar 7, 11:06*am, James Kuyper <jameskuy...@verizon.net> wrote:
> On 03/07/2012 12:42 PM, Nachy wrote:
>
>
>
>
>
> > On Mar 7, 12:32 pm, bert <bert.hutchi...@btinternet.com> 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

Markus Wichmann 03-07-2012 08:26 PM

Re: Compiler warning about comparison always false
 
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

Edward A. Falk 03-07-2012 08:29 PM

Re: Compiler warning about comparison always false
 
In article <c0930b34-a2df-4588-9fb2-1ae3d33785e4@or10g2000pbc.googlegroups.com>,
Nachy <mazwolfe@gmail.com> 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, falk@despams.r.us.com
http://thespamdiaries.blogspot.com/

Keith Thompson 03-07-2012 10:36 PM

Re: Compiler warning about comparison always false
 
falk@rahul.net (Edward A. Falk) writes:
> In article <c0930b34-a2df-4588-9fb2-1ae3d33785e4@or10g2000pbc.googlegroups.com>,
> Nachy <mazwolfe@gmail.com> 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) kst-u@mib.org <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"

Tim Rentsch 03-13-2012 10:30 PM

Re: Compiler warning about comparison always false
 
Philip Lantz <prl@canterey.us> 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.


All times are GMT. The time now is 04:46 PM.

Powered by vBulletin®. Copyright ©2000 - 2014, vBulletin Solutions, Inc.
SEO by vBSEO ©2010, Crawlability, Inc.