On 08/04/2015 07:31 PM, Christopher Li wrote:
On Tue, Aug 4, 2015 at 8:06 AM, Tony Camuso <tcamuso@xxxxxxxxxx> wrote:Missing enum members in case statements in c2xml.c and parse.c were causing compile time complaints by gcc 5.1.1. Better to explicitly handle all enum members of the switch variable in case statements, even if they just break. break; + case SYM_PREPROCESSOR: + case SYM_BASETYPE: + case SYM_NODE: + case SYM_PTR: + case SYM_ARRAY: + case SYM_ENUM: + case SYM_TYPEDEF: + case SYM_TYPEOF: + case SYM_MEMBER: + case SYM_BITFIELD: + case SYM_LABEL: + case SYM_RESTRICT: + case SYM_FOULED: + case SYM_KEYWORD: + case SYM_BAD: + break;I think adding a "default: break" should work. I like your previous patch better, except maybe adding a line break after default.
It was mentioned to me that the new prevailing wisdom is to "Prefer compile time errors to runtime errors", such that each enum'd value of a switch variable should be explicitly handled. I seem to have violated the spirit of that paradigm by using fall through. I could add a new line and "break" statement after each case. Or, if you prefer, I can resubmit the original patch with a linefeed after the "default:" for good form. Either way, I believe the compiler optimize them the same.
case SYM_ENUM: case SYM_RESTRICT: base_type->ident = ident; + case SYM_PREPROCESSOR: + case SYM_BASETYPE:I notice this has the follow through behavior. It is not a bug. But if some one append some new case there, it is easy to cause unintended follow through. It is better add a break there.
It may not be a bug today, but without an explicit break for each, it's a potential bug. The whole reason for explicitly listing the existing enum members is so that new cases for which "break" may not be the correct solution can be easily identified at compile time. Tony -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
