[parted-devel] eliminating switch vs. enum warnings

[Jim Meyering]

Otavio Salvador <otavio at debian.org> wrote:
> Jim Meyering <jim at meyering.net> writes:
>
>> A switch stmt like the above evokes a warning from gcc -Wall
>> about FAT_TYPE_FAT12 not being handled.  One way to fix it
>> is to add an "default:" case, making it equivalent, but warning-free:
>>
>> 	switch (fs_info->fat_type) {
>> 		case FAT_TYPE_FAT16:
>> 			return cluster * 2;
>>
>> 		case FAT_TYPE_FAT32:
>> 			return cluster * 4;
>>
>> 		default:
>> 			break;
>> 	}
>>
>> But what if the FAT_TYPE_FAT12 case is a "can't-happen" case?
>> Add a "PED_ASSERT(0, (void)0)"?
>> or don't bother and just add the "default: break;"?

Thanks for the feedback.

> I think is better to raise the exception so if sometime in future we
> fall there we'll know.

Do you mean the FAT_TYPE_FAT12 case *is* a "should not happen" case?
Are you sure?  If so, I'll be happy to add the following for all of those:

                case FAT_TYPE_FAT12:
                PED_ASSERT (0, (void) 0);
                break;

>> So the above is one class of warned-about switch stmts.
>> The other main one looks like this:
>>
>>         switch (ex_status) {
>>                 case PED_EXCEPTION_CANCEL:
>>                         goto error_close_dev;
>>
>>                 case PED_EXCEPTION_UNHANDLED:
>>                         ped_exception_catch ();
>>                 case PED_EXCEPTION_IGNORE:
>>                         break;
>>         }
>>
>> The problem is that there are numerous other PED_EXCEPTION_* symbols,
>> yet they're essentially "can't happen" cases.  So use the
>> "PED_ASSERT(0, (void)0)" again?  Or just "default: break;"?
>>
>>         switch (ex_status) {
>>                 case PED_EXCEPTION_CANCEL:
>>                         goto error_close_dev;
>>
>>                 case PED_EXCEPTION_UNHANDLED:
>>                         ped_exception_catch ();
>>                 case PED_EXCEPTION_IGNORE:
>>                         break;
>>                 default:
>>                         break;
>>         }
>>
>> My current patch takes the latter (perhaps more conservative)
>> approach, but I can go the other way, too.
>
> I think it's good to us be aware of what has been catch and then an
> exception would help.

Here, it's clear even to me (just learning about this code base)
that the "default:" clause is a "should-not-happen" case.
So I'm adding default blocks like this for each such switch
on PED_EXCEPTION_* values:

            default:
                    PED_ASSERT (0, (void) 0);
                    break;