[Patch] compat ioctl: fix some build warnings

[Patch] compat ioctl: fix some build warnings

[Amerigo Wang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1191 bytes --]

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 <amwang@redhat.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Arnd Bergmann <arnd@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:

Re: [Patch] compat ioctl: fix some build warnings

[Andrew Morton]

On Fri, 5 Feb 2010 02:05:56 -0500
Amerigo Wang <amwang@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 <amwang@redhat.com>
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: Arnd Bergmann <arnd@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

Re: [Patch] compat ioctl: fix some build warnings

[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 <arnd@arndb.de>

Re: [Patch] compat ioctl: fix some build warnings

[Cong Wang]

Andrew Morton wrote:
> On Fri, 5 Feb 2010 02:05:56 -0500
> Amerigo Wang <amwang@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 <amwang@redhat.com>
>> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
>> Cc: Arnd Bergmann <arnd@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
> 

Yeah! Definitely better.

Thanks, Andrew!