Fix gcc build for superio(4)

[lwhsu]

Summary

On https://ci.freebsd.org/job/FreeBSD-head-amd64-gcc/10595/console :

--- all_subdir_superio ---
/workspace/src/sys/modules/superio/../../dev/superio/superio.c: In function 'devtype_to_str':
/workspace/src/sys/modules/superio/../../dev/superio/superio.c:418:1: error: control reaches end of non-void function [-Werror=return-type]
 }
 ^

Test Plan

make -DNO_CLEAN CROSS_TOOLCHAIN=amd64-gcc buildworld buildkernel

[avg]

I did not add the default case, because I wanted a compiler to let me know when I forget to add a new type to that switch.
It sucks that the gcc does not realize that the code after the switch is not reachable.
But I guess that we have to please it.

sys/dev/superio/superio.c

416 ↗

(On Diff #59534)

How about adding a new return statement after the switch instead of 'default label?

[lwhsu]

Agreed, I also think this may be a problem of the old GCC, haven't tried with newer one (which should have been using) yet.

I'll use a new return statement and let's also add a comment that this is only for the old GCC. How do you think?

[avg]

I agree.
Thank you!

[lwhsu]

Update per @avg's suggestion.

[imp]

sys/dev/superio/superio.c

408 ↗

(On Diff #59536)

I'd return "none" or "none-specified" here.

418 ↗

(On Diff #59536)

I'd be tempted to just lose this comment. It adds no value and is wrong.
there's no such thing as an enum type that can have only certain values in C, so the compiler isn't wrong for complaining.
there's no way for the compiler to know this will never be called with an invalid value.

[jhb]

sys/dev/superio/superio.c

418 ↗

(On Diff #59536)

Modern 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.

[lwhsu]

sys/dev/superio/superio.c

418 ↗

(On Diff #59536)

Well, 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"?

[jhb]

sys/dev/superio/superio.c

418 ↗

(On Diff #59536)

Oof, 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.

[imp]

sys/dev/superio/superio.c

418 ↗

(On Diff #59536)

Not 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
%

[jhb]

sys/dev/superio/superio.c

418 ↗

(On Diff #59536)

Actually, 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.

[lwhsu]

• Change string mapping of SUPERIO_DEV_NONE (to distinguish from SUPERIO_DEV_MAX)
• Drop unnecessary comment

[imp]

thanks!