2002-02-07 17:26:45

by Petr Vandrovec

[permalink] [raw]
Subject: Re: [PATCH] read() from driverfs files can read more bytes

On 7 Feb 02 at 8:45, Patrick Mochel wrote:
> On Thu, 7 Feb 2002, Andrey Panin wrote:
> > Attached patch adds check that returned value is less then requested
> > byte count. I know that actual callback function device_read_status()
> > should also be fixed, but I found this bug after midnight and
> > decided to sleep a little :)
>
> That sanity check was in there, once upon a time. However, in moving the
> weight from the driver callbacks to the driverfs read_file() and
> write_file(), it must have got dropped...
>
> Thank you. It's been applied and will be pushed forward.

[I have only 2.5.3 sources here yet]

Can you also check for size >= PAGE_SIZE on enter to entry->show()
procedure? It looks ugly to me that each driver has to check for this
constant unless it wants to smash some innocent kernel memory.

And neither of driverfs_read_file nor driverfs_write_file supports
semantic we use with other filesystems: If at least one byte was
read/written, return byte count (even if error happens). Only if zero
bytes was written, return error code.
Thanks,
Petr Vandrovec
[email protected]


2002-02-07 17:44:05

by Patrick Mochel

[permalink] [raw]
Subject: Re: [PATCH] read() from driverfs files can read more bytes


> Can you also check for size >= PAGE_SIZE on enter to entry->show()
> procedure? It looks ugly to me that each driver has to check for this
> constant unless it wants to smash some innocent kernel memory.

Done. Thanks.

> And neither of driverfs_read_file nor driverfs_write_file supports
> semantic we use with other filesystems: If at least one byte was
> read/written, return byte count (even if error happens). Only if zero
> bytes was written, return error code.

I would think that you would want to return the error code. Say you did:

echo "action parameter" > file

and 'parameter' is an invalid parameter, as determined by the driver. It
would require another arbitrary check to determine if the command
succeeded or not if it returned the number of bytes written. Returning
-EINVAL lets userspace know that it made a boo-boo. Is that not good?

-pat