diff mbox series

mISDN: Fix type of switch control variable in ctrl_teimanager

Message ID 20181019180030.920-1-natechancellor@gmail.com
State Accepted, archived
Delegated to: David Miller
Headers show
Series mISDN: Fix type of switch control variable in ctrl_teimanager | expand

Commit Message

Nathan Chancellor Oct. 19, 2018, 6 p.m. UTC
Clang warns (trimmed for brevity):

drivers/isdn/mISDN/tei.c:1193:7: warning: overflow converting case value
to switch condition type (2147764552 to 18446744071562348872) [-Wswitch]
        case IMHOLD_L1:
             ^
drivers/isdn/mISDN/tei.c:1187:7: warning: overflow converting case value
to switch condition type (2147764550 to 18446744071562348870) [-Wswitch]
        case IMCLEAR_L2:
             ^
2 warnings generated.

The root cause is that the _IOC macro can generate really large numbers,
which don't find into type int. My research into how GCC and Clang are
handling this at a low level didn't prove fruitful and surveying the
kernel tree shows that aside from here and a few places in the scsi
subsystem, everything that uses _IOC is at least of type 'unsigned int'.
Make that change here because as nothing in this function cares about
the signedness of the variable and it removes ambiguity, which is never
good when dealing with compilers.

While we're here, remove the unnecessary local variable ret (just return
-EINVAL and 0 directly).

Link: https://github.com/ClangBuiltLinux/linux/issues/67
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 drivers/isdn/mISDN/tei.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

David Miller Oct. 23, 2018, 2:30 a.m. UTC | #1
From: Nathan Chancellor <natechancellor@gmail.com>
Date: Fri, 19 Oct 2018 11:00:30 -0700

> Clang warns (trimmed for brevity):
> 
> drivers/isdn/mISDN/tei.c:1193:7: warning: overflow converting case value
> to switch condition type (2147764552 to 18446744071562348872) [-Wswitch]
>         case IMHOLD_L1:
>              ^
> drivers/isdn/mISDN/tei.c:1187:7: warning: overflow converting case value
> to switch condition type (2147764550 to 18446744071562348870) [-Wswitch]
>         case IMCLEAR_L2:
>              ^
> 2 warnings generated.
> 
> The root cause is that the _IOC macro can generate really large numbers,
> which don't find into type int. My research into how GCC and Clang are
> handling this at a low level didn't prove fruitful and surveying the
> kernel tree shows that aside from here and a few places in the scsi
> subsystem, everything that uses _IOC is at least of type 'unsigned int'.
> Make that change here because as nothing in this function cares about
> the signedness of the variable and it removes ambiguity, which is never
> good when dealing with compilers.
> 
> While we're here, remove the unnecessary local variable ret (just return
> -EINVAL and 0 directly).
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/67
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>

Applied.
diff mbox series

Patch

diff --git a/drivers/isdn/mISDN/tei.c b/drivers/isdn/mISDN/tei.c
index 12d9e5f4beb1..58635b5f296f 100644
--- a/drivers/isdn/mISDN/tei.c
+++ b/drivers/isdn/mISDN/tei.c
@@ -1180,8 +1180,7 @@  static int
 ctrl_teimanager(struct manager *mgr, void *arg)
 {
 	/* currently we only have one option */
-	int	*val = (int *)arg;
-	int	ret = 0;
+	unsigned int *val = (unsigned int *)arg;
 
 	switch (val[0]) {
 	case IMCLEAR_L2:
@@ -1197,9 +1196,9 @@  ctrl_teimanager(struct manager *mgr, void *arg)
 			test_and_clear_bit(OPTION_L1_HOLD, &mgr->options);
 		break;
 	default:
-		ret = -EINVAL;
+		return -EINVAL;
 	}
-	return ret;
+	return 0;
 }
 
 /* This function does create a L2 for fixed TEI in NT Mode */