advapi32/service.c -- remove untriggerable check

[Robert Shearman]

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