Re: WatchDog Driver Updates

[Russell King <rmk@arm.linux.org.uk>]

On Sat, Apr 06, 2002 at 09:49:57PM -0500, Rob Radez wrote:
> I've put up a patch on http://osinvestor.com/bigwatchdog.diff against
> 2.4.19-pre5-ac3.  The diff is 33k, and it affects 19 files in drivers/char/,

And the purpose of this patch is...

1. copy_to_user is like memcpy() - it takes two pointers and an integer
   size.  You're passing it two integers and a pointer:

   static int wdt977_ioctl(... unsigned long arg)
   ...
            case WDIOC_GETSTATUS:
                   if(copy_to_user(arg, &timer_alive, sizeof(timer_alive)))
                              return -EFAULT;
                   return 0;

2. sizeof(int) != sizeof(unsigned long).  You've changed the ABI in an
   unsafe manner on 64-bit machines.  The ABI is defined by the ioctl
   numbers, which specify an 'int' type.

3. copy_to_user of an int or unsigned long is a bit inefficient.  Why
   not use put_user to write an int or unsigned long?

4. Unless I'm missing something, WDIOC_GETSTATUS can only ever return
   an indication that the timer is open/alive - it's set when the driver
   is opened, and cleared when it is closed.  Since you can't perform
   an ioctl on a closed device, the times that you can perform the ioctl,
   it'll always give you a non-zero result.

5. WDIOC_GETSTATUS is supposed to return the WDIOF_* flags.  Returning
   "watchdog active" as bit 0 causes it to indicate WDIOF_OVERHEAT.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html