92

I am currently working through Accelerated C++ and have come across an issue in exercise 2-3.

A quick overview of the program - the program basically takes a name, then displays a greeting within a frame of asterisks - i.e. Hello ! surrounded framed by *'s.

The exercise - In the example program, the authors use const int to determine the padding (blank spaces) between the greeting and the asterisks. They then ask the reader, as part of the exercise, to ask the user for input as to how big they want the padding to be.

All this seems easy enough, I go ahead ask the user for two integers (int) and store them and change the program to use these integers, removing the ones used by the author, when compiling though I get the following warning;

Exercise2-3.cpp:46: warning: comparison between signed and unsigned integer expressions

After some research it appears to be because the code attempts to compare one of the above integers (int) to a string::size_type, which is fine. But I was wondering - does this mean I should change one of the integers to unsigned int? Is it important to explicitly state whether my integers are signed or unsigned?

 cout << "Please enter the size of the frame between top and bottom you would like ";
 int padtopbottom;
 cin >> padtopbottom;

 cout << "Please enter size of the frame from each side you would like: ";
 unsigned int padsides; 
 cin >> padsides;

 string::size_type c = 0; // definition of c in the program
 if (r == padtopbottom + 1 && c == padsides + 1) { // where the error occurs

Above are the relevant bits of code, the c is of type string::size_type because we do not know how long the greeting might be - but why do I get this problem now, when the author's code didn't get the problem when using const int? In addition - to anyone who may have completed Accelerated C++ - will this be explained later in the book?

I am on Linux Mint using g++ via Geany, if that helps or makes a difference (as I read that it could when determining what string::size_type is).

BartoszKP
  • 34,786
  • 15
  • 102
  • 130
Tim Harrington
  • 923
  • 1
  • 7
  • 4
  • 2
    wouldn't one assume you would want to unsigned ints anyway? I can't think of a logical reason why the top and bottom should be negative – Woot4Moo Sep 07 '10 at 17:08
  • This is true and I mentioned this in the post above, but I still donn't understand why this problem didn't occur in the author's example program when they used const int? I am sure I will get to that in the book, but can't help being curious. – Tim Harrington Sep 07 '10 at 17:23
  • Scrap that - obviously it didn't give a warning in that situation because the int was always going to be 1... oops. – Tim Harrington Sep 07 '10 at 17:37
  • 1
    In general, the increase in range is not worth the hassle of using `unsigned` integral types for counts. Unsigned numbers also have guaranteed wraparound behaviour, making them marginally less efficient. – Jon Purdy Sep 07 '10 at 17:48
  • 5
    The author may have seen the same warning, and just ignored it. Don't assume that the authors of books are any more knowledgeable or careful than the average programmer. – Kristopher Johnson Sep 08 '10 at 17:38

6 Answers6

124

It is usually a good idea to declare variables as unsigned or size_t if they will be compared to sizes, to avoid this issue. Whenever possible, use the exact type you will be comparing against (for example, use std::string::size_type when comparing with a std::string's length).

Compilers give warnings about comparing signed and unsigned types because the ranges of signed and unsigned ints are different, and when they are compared to one another, the results can be surprising. If you have to make such a comparison, you should explicitly convert one of the values to a type compatible with the other, perhaps after checking to ensure that the conversion is valid. For example:

unsigned u = GetSomeUnsignedValue();
int i = GetSomeSignedValue();

if (i >= 0)
{
    // i is nonnegative, so it is safe to cast to unsigned value
    if ((unsigned)i >= u)
        iIsGreaterThanOrEqualToU();
    else
        iIsLessThanU();
}
else
{
    iIsNegative();
}
Kristopher Johnson
  • 81,409
  • 55
  • 245
  • 302
  • 11
    I know that the present C standard sometimes requires that negative signed values compare greater than unsigned values, but should any situations where that occurs not be considered deprecated? I would like to see the standards evolve to at least *permit* compilers to produce arithmetically-correct behavior (meaning that if the signed value is negative, it compares smaller, and if the unsigned value exceeds the maximum value of the signed type, it compares greater). It seems odd that compilers are required to produce goofy behavior in the absence of explicit typecasts. – supercat May 13 '12 at 23:58
  • 7
    @supercat: Since integer comparisons compile to a single machine instruction, and any testing or edge-case handling would need several machine instructions, what you suggest is not likely to be added as a C feature ... it certainly couldn't be the default behavior, as it would needlessly kill performance even when the programmer knows it is not necessary. – Blake Miller Feb 03 '13 at 03:00
  • @BlakeMiller: Code which wants to compare a signed and unsigned value as though both are unsigned could cast one and run "full speed". Otherwise, in many cases the difference would be between a compare-and-jump taking two instructions versus three, which would be cheaper than code which manually handled the various cases. – supercat Feb 03 '13 at 04:11
  • 1
    @BlakeMiller: (The reason I say two versus three is that most code which compares two numbers will use one instruction to perform the compare and set flags based upon them; in many cases, a compiler could arrange things so that prior to the compare, the "sign" flag would hold the upper bit of one of the operands, so a single conditional jump before the compare would suffice to ensure correct semantics). Note that because there are a variety of ways to achieve correct semantics, a compiler could pick whichever one could be done most cheaply. Writing C code for correct semantics would be harder. – supercat Feb 03 '13 at 17:53
  • 7
    Just to demonstrate that "the results can be surprising", the following program (after inserting `#include ` at the top...and I'm using g++ 4.4.7), will print "true", stating that it is true that (signed) -1 is greater than (unsigned) 12: `int main(int, char**) { int x = -1; unsigned int y = 12; printf("x > y: %s\n", x > y ? "true":"false"); return 0; }` – villapx Feb 24 '16 at 22:44
9

I had the exact same problem yesterday working through problem 2-3 in Accelerated C++. The key is to change all variables you will be comparing (using Boolean operators) to compatible types. In this case, that means string::size_type (or unsigned int, but since this example is using the former, I will just stick with that even though the two are technically compatible).

Notice that in their original code they did exactly this for the c counter (page 30 in Section 2.5 of the book), as you rightly pointed out.

What makes this example more complicated is that the different padding variables (padsides and padtopbottom), as well as all counters, must also be changed to string::size_type.

Getting to your example, the code that you posted would end up looking like this:

cout << "Please enter the size of the frame between top and bottom";
string::size_type padtopbottom;
cin >> padtopbottom;

cout << "Please enter size of the frame from each side you would like: ";
string::size_type padsides; 
cin >> padsides;

string::size_type c = 0; // definition of c in the program

if (r == padtopbottom + 1 && c == padsides + 1) { // where the error no longer occurs

Notice that in the previous conditional, you would get the error if you didn't initialize variable r as a string::size_type in the for loop. So you need to initialize the for loop using something like:

    for (string::size_type r=0; r!=rows; ++r)   //If r and rows are string::size_type, no error!

So, basically, once you introduce a string::size_type variable into the mix, any time you want to perform a boolean operation on that item, all operands must have a compatible type for it to compile without warnings.

eric
  • 7,142
  • 12
  • 72
  • 138
6

The important difference between signed and unsigned ints is the interpretation of the last bit. The last bit in signed types represent the sign of the number, meaning: e.g:

0001 is 1 signed and unsigned 1001 is -1 signed and 9 unsigned

(I avoided the whole complement issue for clarity of explanation! This is not exactly how ints are represented in memory!)

You can imagine that it makes a difference to know if you compare with -1 or with +9. In many cases, programmers are just too lazy to declare counting ints as unsigned (bloating the for loop head f.i.) It is usually not an issue because with ints you have to count to 2^31 until your sign bit bites you. That's why it is only a warning. Because we are too lazy to write 'unsigned' instead of 'int'.

AndreasT
  • 9,417
  • 11
  • 46
  • 60
  • Ah I see - I have now changed the counting int as unsigned. Is this considered good practice, or even bad practice? :) – Tim Harrington Sep 07 '10 at 17:35
  • Please, if you downvote, shortly explain why. Even if it is only one word. I can't see anyhting wrong with my answer. Which might be a problem you can help me with. – AndreasT Sep 15 '10 at 08:00
  • 1
    @Tim: "unsigned" is a synonym for "unsigned int". You should use unsigned int or the stl standard counting/iterating variable type std::size_t (which is a synonym as well). It is a best practice to use unsigned in all cases of "iterate over elements 0 to n". It improves clarity and removes warnings, so it is a winner ;-) – AndreasT Sep 15 '10 at 08:06
  • 9
    The internal representation of signed integers is compiler- (i.e. machine-) dependent. Your notation with a sign bit is not widely used due to some problems (+/- zero is one of them). Most machines use a two's complement notion to represent negative numbers. The advantage is that normal (unsigned) arithmetic can be used as well w/o any changes. -1 in 2's complement notion would be 1111 btw. – sstn Jul 19 '11 at 18:49
  • 1
    @AndreasT: while it's understandable to "avoid the whole complement issue for clarity", you could have used an example compatible with 2's complement, the representation used by virtually all platforms. `1001` for -1 was a poor choice, a much better choice would be *"`1111` equals -1 signed and 15 unsigned"* – MestreLion Mar 13 '15 at 19:02
  • Also, it's wrong to say the only difference is the interpretation of the last bit: *all* bits are "interpreted" differently when using 2's notation! The last bit only tells you if a *signed* number is positive or negative. – MestreLion Mar 13 '15 at 19:07
  • _last bit_ is not as clear as other choices as _last_ can refer to the _least_ or _most_ significant. Recommend using _least significant_ for the one's places and _most significant_ for an _unsigned_ most significant bit and _sign bit_ for what you call _last bit_ of _signed_ types.. _Sign bit_ is what the C spec uses. – chux - Reinstate Monica Nov 19 '21 at 04:42
4

At the extreme ranges, an unsigned int can become larger than an int.
Therefore, the compiler generates a warning. If you are sure that this is not a problem, feel free to cast the types to the same type so the warning disappears (use C++ cast so that they are easy to spot).

Alternatively, make the variables the same type to stop the compiler from complaining.
I mean, is it possible to have a negative padding? If so then keep it as an int. Otherwise you should probably use unsigned int and let the stream catch the situations where the user types in a negative number.

Steven Bluen
  • 141
  • 2
  • 11
Martin York
  • 257,169
  • 86
  • 333
  • 562
2

The primary issue is that underlying hardware, the CPU, only has instructions to compare two signed values or compare two unsigned values. If you pass the unsigned comparison instruction a signed, negative value, it will treat it as a large positive number. So, -1, the bit pattern with all bits on (twos complement), becomes the maximum unsigned value for the same number of bits.

8-bits: -1 signed is the same bits as 255 unsigned 16-bits: -1 signed is the same bits as 65535 unsigned etc.

So, if you have the following code:

int fd;
fd = open( .... );

int cnt;
SomeType buf;

cnt = read( fd, &buf, sizeof(buf) );

if( cnt < sizeof(buf) ) {
    perror("read error");
}

you will find that if the read(2) call fails due to the file descriptor becoming invalid (or some other error), that cnt will be set to -1. When comparing to sizeof(buf), an unsigned value, the if() statement will be false because 0xffffffff is not less than sizeof() some (reasonable, not concocted to be max size) data structure.

Thus, you have to write the above if, to remove the signed/unsigned warning as:

if( cnt < 0 || (size_t)cnt < sizeof(buf) ) {
    perror("read error");
}

This just speaks loudly to the problems.

1.  Introduction of size_t and other datatypes was crafted to mostly work, 
    not engineered, with language changes, to be explicitly robust and 
    fool proof.
2.  Overall, C/C++ data types should just be signed, as Java correctly
    implemented.

If you have values so large that you can't find a signed value type that works, you are using too small of a processor or too large of a magnitude of values in your language of choice. If, like with money, every digit counts, there are systems to use in most languages which provide you infinite digits of precision. C/C++ just doesn't do this well, and you have to be very explicit about everything around types as mentioned in many of the other answers here.

0

or use this header library and write:

// |notEqaul|less|lessEqual|greater|greaterEqual
if(sweet::equal(valueA,valueB))

and don't care about signed/unsigned or different sizes

burner
  • 323
  • 2
  • 10