Changeset View
Standalone View
sys/dev/superio/superio.c
Show First 20 Lines • Show All 399 Lines • ▼ Show 20 Lines | static const struct { | ||||
{ 0, 0 } | { 0, 0 } | ||||
}; | }; | ||||
static const char * | static const char * | ||||
devtype_to_str(superio_dev_type_t type) | devtype_to_str(superio_dev_type_t type) | ||||
{ | { | ||||
switch (type) { | switch (type) { | ||||
case SUPERIO_DEV_NONE: | case SUPERIO_DEV_NONE: | ||||
return ("invalid"); | return ("invalid"); | ||||
imp: I'd return "none" or "none-specified" here. | |||||
case SUPERIO_DEV_HWM: | case SUPERIO_DEV_HWM: | ||||
return ("HWM"); | return ("HWM"); | ||||
case SUPERIO_DEV_WDT: | case SUPERIO_DEV_WDT: | ||||
return ("WDT"); | return ("WDT"); | ||||
case SUPERIO_DEV_GPIO: | case SUPERIO_DEV_GPIO: | ||||
return ("GPIO"); | return ("GPIO"); | ||||
case SUPERIO_DEV_MAX: | case SUPERIO_DEV_MAX: | ||||
return ("invalid"); | return ("invalid"); | ||||
Not Done Inline ActionsHow about adding a new return statement after the switch instead of 'default label? avg: How about adding a new return statement after the switch instead of 'default label? | |||||
} | } | ||||
/* Required by old GCC */ | |||||
impUnsubmitted Not Done Inline ActionsI'd be tempted to just lose this comment. It adds no value and is wrong. imp: I'd be tempted to just lose this comment. It adds no value and is wrong.
there's no such thing… | |||||
jhbUnsubmitted Not Done Inline ActionsModern compilers haven't agreed with you in the last 5 years or so. They definitely assume that if you have 'enum foo' you can only ever assign values defined by that type to the enum and not other values. You have to add explicit (int) casts if you need to treat it as a raw int now. So modern compilers will assume that if you've handled all the defined enum values, there won't be a default case. GCC 4.2 is the only compiler capable of compiling FreeBSD that will trip over this. jhb: Modern compilers haven't agreed with you in the last 5 years or so. They definitely assume… | |||||
lwhsuAuthorUnsubmitted Done Inline ActionsWell, but this build is using CROSS_TOOLCHAIN=amd64-gcc, which uses amd64-gcc-6.4.0_5 at this point. Adding a return statement seems trivial to solve the compilation issue here, but is there any way works for @avg 's requirement: "let compiler to report when forgetting to add a new type to that switch"? lwhsu: Well, but this build is using `CROSS_TOOLCHAIN=amd64-gcc`, which uses amd64-gcc-6.4.0_5 at this… | |||||
jhbUnsubmitted Not Done Inline ActionsOof, I had read the comment as only being 4.2. I would also remove the comment if GCC 6 still can't cope. I don't know of a good way to achieve @avg's requirement. We could maybe put the return under #ifndef clang but that's kind of ugly. jhb: Oof, I had read the comment as only being 4.2. I would also remove the comment if GCC 6 still… | |||||
jhbUnsubmitted Done Inline ActionsActually, no need for the #ifndef, I forgot the return wasn't in a default: so it should keep the warning @avg cares about. I would just drop the comment though since it isn't just GCC 4.2. jhb: Actually, no need for the #ifndef, I forgot the return wasn't in a default: so it should keep… | |||||
impUnsubmitted Not Done Inline ActionsNot sure that you are correct John: % cat foo.c enum foo { a = 1, b = 2, c = 4 }; enum foo x; void bar(void) { x = a | c; } % clang -Wall foo.c -c % clang -v clang -v FreeBSD clang version 8.0.0 (tags/RELEASE_800/final 356365) (based on LLVM 8.0.0) Target: x86_64-unknown-freebsd13.0 Thread model: posix InstalledDir: /usr/bin % imp: Not sure that you are correct John:
```
% cat foo.c
enum foo { a = 1, b = 2, c = 4 };
enum foo… | |||||
return ("invalid"); | |||||
} | } | ||||
static int | static int | ||||
superio_detect(device_t dev, bool claim, struct siosc *sc) | superio_detect(device_t dev, bool claim, struct siosc *sc) | ||||
{ | { | ||||
struct resource *res; | struct resource *res; | ||||
rman_res_t port; | rman_res_t port; | ||||
rman_res_t count; | rman_res_t count; | ||||
▲ Show 20 Lines • Show All 528 Lines • Show Last 20 Lines |
I'd return "none" or "none-specified" here.