Al Viro wrote:
On Tue, May 02, 2006 at 04:52:53PM +0300, Avi Kivity wrote:
static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos,
size_t count, loff_t max)
{
loff_t pos;
ssize_t retval;
/*
* Get input file, and verify that it is ok..
*/
light_file_ptr in_file(in_fd);
*snerk*
Good luck defining copying and conversion to file * for that puppy.
class light_file_ptr {
public:
explicit light_file_ptr(int fd)
{
_file = fget_light(fd, &_fput_needed);
}
~light_file_ptr()
{
fput_light(_file, _fput_needed);
}
bool valid() const
{
return _file != 0;
}
struct file *operator->() // allowed for libs :)
{
return _file;
}
private:
struct file *_file;
int _fput_needed;
};
struct inode *in_inode = in_file->dentry()->inode();
Lovely. Let's expose all fields as methods?
If you like. I won't insist.
if (!in_inode)
return -EINVAL;
BTW, that can't happen. Applies to the original as well.
I believe this bug is more visible when there is less code in the function.
// I'm assuming here that the default sendfile() returns -EINVAL
if (!ppos)
ppos = &in_file->f_pos;
else
if (!(in_file->mode() & FMODE_PREAD))
return -ESPIPE;
As opposed to ->readable() for checking FMODE_READ?
Forgot, sorry. I'll redo the patch.
light_file_ptr out_file(out_fd);
if (!out_file)
return -EBADF;
?
Sorry, !out_file.valid().
if (!max)
max = min(in_inode->i_sb->s_maxbytes,
out_inode->i_sb->s_maxbytes);
While we are at it, that's the only place where in_inode and out_inode
are used. Also... how does one remember which of ->dentry, ->inode
and ->i_sb are methods and which are public fields?
I usually make all publics either methods (a class) or fields (a struct).
// now, with exceptions
static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos,
size_t count, loff_t max)
{
loff_t pos;
/*
* Get input file, and verify that it is ok..
*/
light_file_ptr in_file(in_fd);
in_file->verify_readable();
That assumes that error value returned in that case is the same everywhere.
It isn't.
Okay, in_file->verify_readable(EBADF); Yuck.
Thanks for the review.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
