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

[Gerald Pfeifer]

For the record, this is how this has been addressed a few months
after this discussion.  That's another way of doing it. :-)

Gerald



commit 1af17844301c4924dd8324cbf2f9c3c1ef0394b2
Author: Huw Davies <huw at codeweavers.com>
Date:   Tue May 4 14:05:13 2010 +0100

    comctl32: Silence a few compiler warnings.

diff --git a/dlls/comctl32/datetime.c b/dlls/comctl32/datetime.c
index dc2dd12..2418950 100644
--- a/dlls/comctl32/datetime.c
+++ b/dlls/comctl32/datetime.c
@@ -614,7 +614,7 @@ DATETIME_ReturnFieldWidth (const DATETIME_INFO 
*infoPtr, HDC hdc, int count, SHO
                    lctype = LOCALE_SABBREVMONTHNAME1;
                    max_count = 12;
                    break;
-               case FULLMONTH:
+               default: /* FULLMONTH */
                    fall   = fld_mon;
                    lctype = LOCALE_SMONTHNAME1;
                    max_count = 12;


On Fri, 25 Dec 2009, James McKenzie wrote:
> 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