Uwe wrote:
Gerald Pfeifer writes:
n is of type DWORD which is unsigned, so n < 0 always will evaluate to false.
Is dropping the check the right answer? Shouldn't the check test for high values like > 0xff00 and report a possible problem?
Indeed. IMHO we don't need patches like this, and Gerald is not thinking hard enough. Simplistic just-remove-the-warning patches are a Bad Thing, and we Don't Want To Encourage Them. Please stop. - Dan
Dan Kegel wrote:
Uwe wrote:
Gerald Pfeifer writes:
n is of type DWORD which is unsigned, so n < 0 always will evaluate to false.
Is dropping the check the right answer? Shouldn't the check test for high values like > 0xff00 and report a possible problem?
Indeed. IMHO we don't need patches like this, and Gerald is not thinking hard enough. Simplistic just-remove-the-warning patches are a Bad Thing, and we Don't Want To Encourage Them. Please stop.
Arguably, the check shouldn't be there anyway. Either the code always calculates the buffer size required correctly or it doesn't. As Gerald has pointed out, the extra check doesn't actually trigger so it is useless and makes people have a false sense of security in the code.
I'm not a big fan of turning on obscure warnings in gcc, but it has uncovered some real bugs and Gerald has argued before that uncovering real bugs is easier when there are less harmless warnings to sift through. Obviously, if the code becomes less readable through this process or it introduces bugs then this is not desirable, but that isn't the case with this patch.
-
Dan Kegel -
Robert Shearman