Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#7111 closed Feature Requests (fixed)

Switch case's default missing in date_time

Reported by: Gaurav Gupta <g.gupta@…> Owned by: Marshall Clow
Milestone: To Be Determined Component: date_time
Version: Boost 1.51.0 Severity: Cosmetic
Keywords: Cc: yogen.saini@…, viboes

Description

In file boost/date_time/date_formatting.hpp in following code block, default case of switch condition is missing.Switch case should not be used without default case ideally.In case of default, code should break from the loop.

static ostream_type& format_month(const month_type& month,
                                      ostream_type &os)
    {
      switch (format_type::month_format()) 
      {
        case month_as_short_string: 
        { 
          os << month.as_short_string(); 
          break;
        }
        case month_as_long_string: 
        { 
          os << month.as_long_string(); 
          break;
        }
        case month_as_integer: 
        { 
          os << std::setw(2) << std::setfill(os.widen('0')) << month.as_number();
          break;
        }
     
      }
      return os;
    } // format_month
  };

Attached patch is the fix for it.

Attachments (1)

date_formatting.hpp_patch (430 bytes ) - added by Gaurav Gupta <g.gupta@…> 10 years ago.
Fix for the reported Bug.

Download all attachments as: .zip

Change History (11)

by Gaurav Gupta <g.gupta@…>, 10 years ago

Attachment: date_formatting.hpp_patch added

Fix for the reported Bug.

comment:1 by Marshall Clow, 10 years ago

I'm sorry, but I don't see the point of this change.

The code here is checking for all the possible values of the month_format_spec enum; there are no other (legal) values.

Also, your patch does not change the behavior of the code; if an invalid value was returned the by month_format(), the current code would do nothing, and your patch would change it so that it would ... do nothing.

Please tell me what you are trying to accomplish here.

P.S. I am completely missing what you mean by:

In case of default, code should break from the loop.

since I don't see a loop here.

comment:2 by Gaurav Gupta <g.gupta@…>, 10 years ago

Severity: ProblemCosmetic

I just want to highlight the coding practices. Its a cosmetic bug. In future if somebody add more cases, it will be easier for the user as code will be more readable and behavior of switch will be visible. Its obvious that it is still not doing anything, but the readability is more and it is according to coding guidelines.

comment:3 by viboes, 10 years ago

Cc: viboes added
Resolution: invalid
Status: newclosed
Type: BugsFeature Requests

There are too much work to do to clean up Boost.DateTime. Re-open if some tool is warning a bad usage.

comment:4 by Gaurav Gupta <g.gupta@…>, 10 years ago

Resolution: invalid
Status: closedreopened

If our application is using "Werror" flag, then if default case is not mentioned ,it is reported as an error.

comment:5 by Marshall Clow, 10 years ago

Ok; I added a default case. But you should bug the gcc folks, since it's a bogus warning.

Last edited 10 years ago by Marshall Clow (previous) (diff)

comment:6 by Marshall Clow, 10 years ago

(In [80703]) Added default case label to silence bogus warning; Refs #7111

comment:7 by Marshall Clow, 10 years ago

Owner: changed from az_sw_dude to Marshall Clow
Status: reopenednew

comment:8 by Gaurav Gupta <g.gupta@…>, 10 years ago

Thanks Marshall, Please commit similar warning errors in Ticket# 7112 & 7113.

comment:9 by Marshall Clow, 10 years ago

Resolution: fixed
Status: newclosed

(In [80797]) Merge bug fixes to release; Fixes #5550 Fixes #6136 Fixes #6513 Fixes #7111 Fixes #7112 Fixes #7113 Fixes #7342 Fixes #7426

comment:10 by Marshall Clow, 10 years ago

BTW, clang now warns on this code.

Warning: default label in switch which covers all enumeration values [ -Wcovered-switch-default ]
Note: See TracTickets for help on using tickets.