Meaning of int n != unsigned int(n)

Question

I was reading a code segment that was checking if some values from a ppm file are correct. The values were read like this:

image >> img_format >> width >> height >> maxval;

And then to check if width and height are correct this was used:

if (width != unsigned int(width) && height != unsigned int(height))
{
    cerr << "Width and/or height of the image are missing." << endl;
    exit(EXIT_FAILURE);
}

**width and height are declared as unsigned int

How does this condition work?

This question isn't really complete unless you show how `width` and `height` are declared. — lurker, Nov 30 '18 at 11:45
I suppose `if ( width < 0 )` wasn't obscure enough? Interestingly enough, it appears to be an illegal syntax: https://stackoverflow.com/questions/40765232/why-can-constructor-syntax-not-be-used-with-the-unsigned-int-type — Andrew Henle, Nov 30 '18 at 11:49
And given the edit "`width` and `height` are declared as `unsigned int`", the dubious functional cast to `unsigned int` seems, well, utterly pointless. The full context and history of this code might be highly relevant to what's going on here, as it looks like someone may have been flailing about trying to validate input. — Andrew Henle, Nov 30 '18 at 11:59
From https://stackoverflow.com/questions/2711522/what-happens-if-i-assign-a-negative-value-to-an-unsigned-variable even if the syntax WAS correct, there's no guarantee that the number will be different if it was negative. — UKMonkey, Nov 30 '18 at 12:06
`unsigned int (with)` is not a valid functional cast expression. The only effect of this code will be to make it not compilable with anything else than MSVC — Oliv, Nov 30 '18 at 12:06
The answer to "How does this condition work?" is "it doesn't" but I suspect the full answer needs to include the why. — UKMonkey, Nov 30 '18 at 12:11
When you read code like this then the correct interpretation is "uh oh, there might be bugs here that haven't been diagnosed yet". If you have the option to not use it then you should take it. — Hans Passant, Nov 30 '18 at 12:21
It probably means whoever wrothe it didn't quite know what they were doing. — n. m. could be an AI, Nov 30 '18 at 13:26
@n.m. See the discussion below. I'd venture that the original code used `int` and not `unsigned int` and the functional cast to `unsigned int` was an attempt to detect invalid input (yes, `width < 0` would be so much better for a lot of reasons, most important that it actually works...), and that a **later** modification to `unsigned int` was done. Just a guess, though it's one informed by a lot of experience with bad code like this - way too much of it mine, all from my younger days - I no longer make coding mistakes. ;-) — Andrew Henle, Nov 30 '18 at 15:23
@AndrewHenle notice the word "missing" in the diagnostic output. Why do you think it's there? — n. m. could be an AI, Nov 30 '18 at 16:05

score 0 · Answer 1 · answered Nov 30 '18 at 18:51

0

Unsigned int means that the number is 0 or any positive number. In your case, it is checking that both the variables width and height are 0 or some positive integer.

Side Note: Size_t is another datatype that represents unsigned integers.

answered Nov 30 '18 at 18:51

Michelle

265
2
14

score -1 · Answer 2 · edited Nov 30 '18 at 14:37

-1

First of all unsigned int(width) is illegal syntax. Maybe there is an extension to a compiler allowing it (i've checked both clang and gcc and neither allow it, even with gnu++). There are ways to correctly write this cast:

(unsigned int)(width)
unsigned(width)
static_cast<unsigned int>(width)
static_cast<unsigned(width)

The static_casts are recommended.

Now assuming one of the above way of writing your condition it does nothing. You way witdth and height are of type unsigned int so the cast does absolutely nothing thus the comparison is equivalent to:

width != width

Which will always be false.

So it is a bug.

The correct way to check if the stream reading operation was successful or not is:

if (!(image >> img_format >> width >> height >> maxval))
    // error

or equivalent:

image >> img_format >> width >> height >> maxval;

if (!image)
    // error

edited Nov 30 '18 at 14:37

Toby Speight

27,591
48
66
103

answered Nov 30 '18 at 13:47

bolov

72,283
15
145
224

2

Rather than "whoever wrote it had no idea what he/she was doing", I find it more likely that `width`/`height` used to be non-`unsigned`, and this code was not updated. Another reason not to use such "tricks" as they tend to get buried. – Lightness Races in Orbit Nov 30 '18 at 14:16
@LightnessRacesinOrbit good point. I was a bad assumption on my part. – bolov Nov 30 '18 at 14:25
@LightnessRacesinOrbit but even if `width` is `int` then the comparison is a constant. We don't don't know what happened so let's say it's a bug and leave it at that. – bolov Nov 30 '18 at 14:28
1

I guess what I'm saying is... since we don't know what happened perhaps we should keep the aspersions to a minimum :) – Lightness Races in Orbit Nov 30 '18 at 14:53
@bolov in Visual C++ It is actually a **valid syntax** . – Marco D.G. Nov 30 '18 at 14:58
@LightnessRacesinOrbit: If `width` and `height` used to be ordinary `signed int`, then this code is even stronger evidence that the author didn't know what he was doing. (By which I mean that "what he's doing" is causing silent promotion of the signed operand to an unsigned type, meaning that the comparison works on two identically-converted values, and no one who knew that was happening would ever write this code) – Ben Voigt Nov 30 '18 at 14:58
@LightnessRacesinOrbit Blindly changing a type just makes it second-order not-knowing-what-you're-doing. Heck, how does `istream::operator>>(unsigned int &x);` even treat a negative input? Hence my "flailing about" comment to the original question. – Andrew Henle Nov 30 '18 at 14:59
@BenVoigt Is that mandated by the standard? https://stackoverflow.com/questions/7677158/how-to-detect-negative-numbers-as-parsing-errors-when-reading-unsigned-integers seems relevant. – Andrew Henle Nov 30 '18 at 15:01
1

@AndrewHenle: Ahh, the Standard says the behavior is the same as `strtoull` and *that* is defined by the rules of the C standard, and says "If the minus sign was part of the input sequence, the numeric value calculated from the sequence of digits is negated as if by unary minus in the result type, which applies unsigned integer wraparound rules." – Ben Voigt Nov 30 '18 at 15:09
@BenVoigt Which means, assuming someone changed the original `int` to `unsigned int`, that the change to `unsigned int` broke the ability to detect invalid negative input (nevermind that the check itself was flawed...). – Andrew Henle Nov 30 '18 at 15:14
@MarcoD.G. *in Visual C++ It is actually a **valid syntax** .* Being accepted by Visual C++ doesn't make it valid syntax. `unsigned int` is not a "simple type specifier" per **7.1.5.2 Simple type specifiers** of the C++ standard. Only simple type specifiers are valid per **5.2.3 Explicit type conversion (functional notation)**, paragraph 1: "A **simple-type-specifier** (7.1.5) followed by a parenthesized *expression-list* constructs a value of the specified type given the expression list. ..." – Andrew Henle Nov 30 '18 at 15:18

Meaning of int n != unsigned int(n)

2 Answers2