[Patch] compat ioctl: fix some build warnings

[Amerigo Wang]

I got these warnings:

fs/compat_ioctl.c: In function ‘compat_sys_ioctl':
fs/compat_ioctl.c:534: warning: ‘karg' may be used uninitialized in this function
fs/compat_ioctl.c:533: warning: ‘kcmd' may be used uninitialized in this function
fs/compat_ioctl.c:656: warning: ‘ret' may be used uninitialized in this function

These warnings are harmless, so just shut gcc up.

Signed-off-by: WANG Cong <amw...@redhat.com>
Cc: Alexander Viro <vi...@zeniv.linux.org.uk>
Cc: Arnd Bergmann <ar...@arndb.de>

---
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index c5c45de..2468f80 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -543,6 +543,8 @@ static int mt_ioctl_trans(unsigned int fd, unsigned int cmd, void __user *argp)
kcmd = MTIOCGET;
karg = &get;
break;
+ default:
+ return -EINVAL;
}
set_fs (KERNEL_DS);
err = sys_ioctl (fd, kcmd, (unsigned long)karg);
@@ -653,7 +655,7 @@ static int set_raw32_request(struct raw_config_request *req, struct raw32_config
static int raw_ioctl(unsigned fd, unsigned cmd,
struct raw32_config_request __user *user_req)
{
- int ret;
+ int ret = -EINVAL;

switch (cmd) {
case RAW_SETBIND:
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Andrew Morton]

On Fri, 5 Feb 2010 02:05:56 -0500
Amerigo Wang <amw...@redhat.com> wrote:

> I got these warnings:
>
> fs/compat_ioctl.c: In function ___compat_sys_ioctl':
> fs/compat_ioctl.c:534: warning: ___karg' may be used uninitialized in this function
> fs/compat_ioctl.c:533: warning: ___kcmd' may be used uninitialized in this function
> fs/compat_ioctl.c:656: warning: ___ret' may be used uninitialized in this function

>
> These warnings are harmless, so just shut gcc up.
>
> Signed-off-by: WANG Cong <amw...@redhat.com>
> Cc: Alexander Viro <vi...@zeniv.linux.org.uk>
> Cc: Arnd Bergmann <ar...@arndb.de>
>
> ---
> diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
> index c5c45de..2468f80 100644
> --- a/fs/compat_ioctl.c
> +++ b/fs/compat_ioctl.c
> @@ -543,6 +543,8 @@ static int mt_ioctl_trans(unsigned int fd, unsigned int cmd, void __user *argp)
> kcmd = MTIOCGET;
> karg = &get;
> break;
> + default:
> + return -EINVAL;
> }
> set_fs (KERNEL_DS);
> err = sys_ioctl (fd, kcmd, (unsigned long)karg);
> @@ -653,7 +655,7 @@ static int set_raw32_request(struct raw_config_request *req, struct raw32_config
> static int raw_ioctl(unsigned fd, unsigned cmd,
> struct raw32_config_request __user *user_req)
> {
> - int ret;
> + int ret = -EINVAL;
>
> switch (cmd) {
> case RAW_SETBIND:

Unfortunately that adds more code for something which cannot happen at
runtime.

I guess we could do this trick:

--- a/fs/compat_ioctl.c~a
+++ a/fs/compat_ioctl.c
@@ -539,7 +539,7 @@ static int mt_ioctl_trans(unsigned int f
kcmd = MTIOCPOS;
karg = &pos;
break;
- case MTIOCGET32:
+ default: /* MTIOCGET32 */

kcmd = MTIOCGET;
karg = &get;
break;

@@ -657,7 +657,7 @@ static int raw_ioctl(unsigned fd, unsign

switch (cmd) {
case RAW_SETBIND:
- case RAW_GETBIND: {
+ default: { /* RAW_GETBIND */
struct raw_config_request req;
mm_segment_t oldfs = get_fs();

_

Which actually reduces code:

akpm:/usr/src/25> size fs/compat_ioctl.o
text data bss dec hex filename
9012 2232 2864 14108 371c fs/compat_ioctl.o

8968 2232 2848 14048 36e0 fs/compat_ioctl.o

[Arnd Bergmann]

On Friday 05 February 2010, Andrew Morton wrote:
> Unfortunately that adds more code for something which cannot happen at
> runtime.
>
> I guess we could do this trick:
>
> --- a/fs/compat_ioctl.c~a
> +++ a/fs/compat_ioctl.c
> @@ -539,7 +539,7 @@ static int mt_ioctl_trans(unsigned int f
> kcmd = MTIOCPOS;
> karg = &pos;
> break;
> - case MTIOCGET32:
> + default: /* MTIOCGET32 */
> kcmd = MTIOCGET;
> karg = &get;
> break;
> @@ -657,7 +657,7 @@ static int raw_ioctl(unsigned fd, unsign
>
> switch (cmd) {
> case RAW_SETBIND:
> - case RAW_GETBIND: {
> + default: { /* RAW_GETBIND */
> struct raw_config_request req;
> mm_segment_t oldfs = get_fs();
>

Looks good to me. Actually, we could just kill the switch/case
statement in raw_ioctl entirely, but I wouldn't bother at this
point any more because there are already patches from both Al
and me to kill that function by moving it into drivers/char/raw.c

I need to check what happened to that patch and to the other
patches I have removing code form fs/compat_ioctl.c, they might
need to be updated before I can submit them for the next merge.

In the meantime,

Acked-by: Arnd Bergmann <ar...@arndb.de>

[Cong Wang]

Yeah! Definitely better.

Thanks, Andrew!