Improve handling of invalid input in dlls/comctl32/datetime.c

[James McKenzie]

Nikolay Sivov wrote:
> On 12/26/2009 00:04, James McKenzie wrote:
>> Nikolay Sivov wrote:
>>   
>>> On 12/25/2009 14:18, Gerald Pfeifer wrote:
>>>     
>>>> Otherwise max_count will be undefined in the following loop may do
>>>> interesting things it seems.  (Does Coverity diagnose similar items?)
>>>>
>>>> Gerald
>>>>
>>>> ChangeLog:
>>>> Improve handling of invalid input in DATETIME_ReturnFieldWidth.
>>>>
>>>> diff --git a/dlls/comctl32/datetime.c b/dlls/comctl32/datetime.c
>>>> index c240d4f..08c7082 100644
>>>> --- a/dlls/comctl32/datetime.c
>>>> +++ b/dlls/comctl32/datetime.c
>>>> @@ -38,6 +38,7 @@
>>>>     *    -- FORMATCALLBACK
>>>>     */
>>>>
>>>> +#include<assert.h>
>>>>    #include<math.h>
>>>>    #include<string.h>
>>>>    #include<stdarg.h>
>>>> @@ -619,6 +620,9 @@ DATETIME_ReturnFieldWidth (const DATETIME_INFO
>>>> *infoPtr, HDC hdc, int count, SHO
>>>>                lctype = LOCALE_SMONTHNAME1;
>>>>                max_count = 12;
>>>>                break;
>>>> +        default:
>>>> +            assert(0);
>>>> +            max_count=0;
>>>>            }
>>>>
>>>>            cx = 0;
>>>>
>>>>        
>>> Hi, Gerald.
>>>
>>> This can't even happen. spec if filtered later with:
>>> ---
>>>          case THREECHARMONTH:
>>>          case FULLMONTH:
>>>          case THREECHARDAY:
>>>          case FULLDAY:
>>> ---
>>> So no default case here.
>>>
>>>      
>> Even if the default case cannot be reached, it is not a 'bad thing' to
>> have one, even if it is an error message stating "Something went wrong,
>> we should not get here" with an exit code...
>>    
> Why? There's no default case, treat is as 'if () {} else if () {} etc.'.
> It's the same thing to have explicit initializers for all local
> variables even if I don't use it before
> set some value. It's a pollution, especially if used to silence
> analyzer warnings.
Nikolay:

Things can and do go 'wrong'.  For instance, you build on Linux, I build
on a Mac.  Sometimes the ported programs from UNIX can and will do
'strange' things.  If you don't have a method to know that there is an
error, then you go about blaming Wine when it is a poor program port. 
Also, it is good programming practice to account for the situations you
don't expect to encounter and to initialize variables that you will be
using.  This is not a 'just in case' thing, it can help when program
errors occur to see where in the code an expected value did or did not
happen. 

I know, it looks like pollution, but when code goes wrong, and it will,
this makes troubleshooting much easier.  I know this from running
Quality Assurance testing for many years and coding myself.  My
programming instructor deducted grade points for failing to handle error
and other unexpected conditions.

James McKenzie