advapi32/service.c -- remove untriggerable check
Robert Shearman
rob at codeweavers.com
Sun Nov 18 13:21:31 CST 2007
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.
--
Rob Shearman
More information about the wine-devel
mailing list