[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <200906111409.23246.paul.moore@hp.com>
Date: Thu, 11 Jun 2009 14:09:22 -0400
From: Paul Moore <paul.moore@...com>
To: Arnaldo Carvalho de Melo <acme@...hat.com>
Cc: David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
Chris Van Hoof <vanhoof@...hat.com>,
Clark Williams <williams@...hat.com>,
Caitlin Bestler <caitlin.bestler@...il.com>,
Steven Whitehouse <steve@...gwyn.com>,
" RĂ©mi Denis-Courmont"
<remi.denis-courmont@...ia.com>,
Neil Horman <nhorman@...driver.com>,
Nivedita Singhvi <niv@...ibm.com>
Subject: Re: [RFC v2] net: Introduce recvmmsg socket syscall
On Wednesday 10 June 2009 11:40:22 pm Arnaldo Carvalho de Melo wrote:
> diff --git a/net/socket.c b/net/socket.c
> index 791d71a..f9f1e20 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -702,6 +702,28 @@ int sock_recvmsg(struct socket *sock, struct msghdr
> *msg, return ret;
> }
>
> +static int sock_recvmsg_nosec(struct socket *sock, struct msghdr *msg,
> + size_t size, int flags)
> +{
> + struct kiocb iocb;
> + struct sock_iocb siocb;
> + int ret;
> +
> + init_sync_kiocb(&iocb, NULL);
> + iocb.private = &siocb;
> +
> + siocb.sock = sock;
> + siocb.scm = NULL;
> + siocb.msg = msg;
> + siocb.size = size;
> + siocb.flags = flags;
> +
> + ret = sock->ops->recvmsg(&iocb, sock, msg, size, flags);
> + if (-EIOCBQUEUED == ret)
> + ret = wait_on_sync_kiocb(&iocb);
> + return ret;
> +}
Hmmm, in an effort to reduce duplicated code how about updating
__sock_recvmsg() to something like the following:
static inline int __sock_recvmsg(struct kiocb *iocb, struct socket *sock,
struct msghdr *msg, size_t size, int flags)
{
int err;
err = security_socket_recvmsg(...);
if (err)
return err;
return sock_recvmsg_nosec(...);
}
The only real difference is that now the *_kiocb() functions get called and I
have no clue if that is good or bad but it is different :)
> /*
> @@ -2018,46 +2029,47 @@ SYSCALL_DEFINE3(recvmsg, int, fd, struct msghdr
> __user *, msg, * kernel msghdr to use the kernel address space)
> */
>
> - uaddr = (__force void __user *)msg_sys.msg_name;
> + uaddr = (__force void __user *)msg_sys->msg_name;
> uaddr_len = COMPAT_NAMELEN(msg);
> if (MSG_CMSG_COMPAT & flags) {
> - err = verify_compat_iovec(&msg_sys, iov,
> + err = verify_compat_iovec(msg_sys, iov,
> (struct sockaddr *)&addr,
> VERIFY_WRITE);
> } else
> - err = verify_iovec(&msg_sys, iov,
> + err = verify_iovec(msg_sys, iov,
> (struct sockaddr *)&addr,
> VERIFY_WRITE);
> if (err < 0)
> goto out_freeiov;
> total_len = err;
>
> - cmsg_ptr = (unsigned long)msg_sys.msg_control;
> - msg_sys.msg_flags = flags & (MSG_CMSG_CLOEXEC|MSG_CMSG_COMPAT);
> + cmsg_ptr = (unsigned long)msg_sys->msg_control;
> + msg_sys->msg_flags = flags & (MSG_CMSG_CLOEXEC|MSG_CMSG_COMPAT);
>
> if (sock->file->f_flags & O_NONBLOCK)
> flags |= MSG_DONTWAIT;
> - err = sock_recvmsg(sock, &msg_sys, total_len, flags);
> + err = (nosec ? sock_recvmsg_nosec : sock_recvmsg)(sock, msg_sys,
> + total_len, flags);
Perhaps I'm just being nit-picky here but why not this (it is much easier on
my eyes at least <g>):
if (nosec)
err = sock_recvmsg_nosec(...);
else
err = sock_recvmsg(...);
--
paul moore
linux @ hp
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists