[PATCH -mm] idetape gcc 4.1 warning fix

[PATCH -mm] idetape gcc 4.1 warning fix

[Daniel Walker]

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

Fixes the following warning,

drivers/ide/ide-tape.c: In function ‘idetape_copy_stage_from_user’:
drivers/ide/ide-tape.c:2662: warning: ignoring return value of ‘copy_from_user’, declared with attribute warn_unused_result
drivers/ide/ide-tape.c: In function ‘idetape_copy_stage_to_user’:
drivers/ide/ide-tape.c:2689: warning: ignoring return value of ‘copy_to_user’, declared with attribute warn_unused_result

Signed-Off-By: Daniel Walker <dwalker@mvista.com>

Index: linux-2.6.16/drivers/ide/ide-tape.c
===================================================================
--- linux-2.6.16.orig/drivers/ide/ide-tape.c
+++ linux-2.6.16/drivers/ide/ide-tape.c
@@ -2659,7 +2659,7 @@ static void idetape_copy_stage_from_user
 		}
 #endif /* IDETAPE_DEBUG_BUGS */
 		count = min((unsigned int)(bh->b_size - atomic_read(&bh->b_count)), (unsigned int)n);
-		copy_from_user(bh->b_data + atomic_read(&bh->b_count), buf, count);
+		WARN_ON(copy_from_user(bh->b_data + atomic_read(&bh->b_count), buf, count));
 		n -= count;
 		atomic_add(count, &bh->b_count);
 		buf += count;
@@ -2686,7 +2686,7 @@ static void idetape_copy_stage_to_user (
 		}
 #endif /* IDETAPE_DEBUG_BUGS */
 		count = min(tape->b_count, n);
-		copy_to_user(buf, tape->b_data, count);
+		WARN_ON(copy_to_user(buf, tape->b_data, count));
 		n -= count;
 		tape->b_data += count;
 		tape->b_count -= count;

Re: [PATCH -mm] idetape gcc 4.1 warning fix

[Alan Cox]

On Maw, 2006-05-09 at 19:55 -0700, Daniel Walker wrote:
> Fixes the following warning,
> 
> drivers/ide/ide-tape.c: In function ‘idetape_copy_stage_from_user’:
> drivers/ide/ide-tape.c:2662: warning: ignoring return value of ‘copy_from_user’, declared with attribute warn_unused_result
> drivers/ide/ide-tape.c: In function ‘idetape_copy_stage_to_user’:

>  		count = min((unsigned int)(bh->b_size - atomic_read(&bh->b_count)), (unsigned int)n);
> -		copy_from_user(bh->b_data + atomic_read(&bh->b_count), buf, count);
> +		WARN_ON(copy_from_user(bh->b_data + atomic_read(&bh->b_count), buf, count));

So you want to let users spew all over the kernel log when a copy from
user fails. Either fix it properly or leave it alone. In this case its
actually quite hard to fix properly which is why it hasn't been done.

(POSIX doesn't require invalid addresses reliably report -EFAULT)

Re: [PATCH -mm] idetape gcc 4.1 warning fix

[Jakub Jelinek]

On Tue, May 09, 2006 at 07:55:58PM -0700, Daniel Walker wrote:
> Fixes the following warning,
> 
> drivers/ide/ide-tape.c: In function ‘idetape_copy_stage_from_user’:
> drivers/ide/ide-tape.c:2662: warning: ignoring return value of ‘copy_from_user’, declared with attribute warn_unused_result
> drivers/ide/ide-tape.c: In function ‘idetape_copy_stage_to_user’:
> drivers/ide/ide-tape.c:2689: warning: ignoring return value of ‘copy_to_user’, declared with attribute warn_unused_result
> 
> Signed-Off-By: Daniel Walker <dwalker@mvista.com>
> 
> Index: linux-2.6.16/drivers/ide/ide-tape.c
> ===================================================================
> --- linux-2.6.16.orig/drivers/ide/ide-tape.c
> +++ linux-2.6.16/drivers/ide/ide-tape.c
> @@ -2659,7 +2659,7 @@ static void idetape_copy_stage_from_user
>  		}
>  #endif /* IDETAPE_DEBUG_BUGS */
>  		count = min((unsigned int)(bh->b_size - atomic_read(&bh->b_count)), (unsigned int)n);
> -		copy_from_user(bh->b_data + atomic_read(&bh->b_count), buf, count);
> +		WARN_ON(copy_from_user(bh->b_data + atomic_read(&bh->b_count), buf, count));

WARN_ON is certainly not a good way to hide this warning.
Having a user-triggerable WARN_ON is a bad idea.
Instead you should add some error handling.

	Jakub

Re: [PATCH -mm] idetape gcc 4.1 warning fix

[Daniel Walker]

On Wed, 2006-05-10 at 07:12 -0400, Jakub Jelinek wrote:

> WARN_ON is certainly not a good way to hide this warning.
> Having a user-triggerable WARN_ON is a bad idea.
> Instead you should add some error handling.

This was one of 3 or so functions that had no return facility . I had a
feeling people wouldn't agree with the WARN_ON() , so I'm hoping someone
has a better solution . Could just be a printk , but I'm not going to
revamp the drivers error handling just to cure the warning .

Daniel