| Message ID | 1376536816-10951-1-git-send-email-asias@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
From: Asias He <asias@redhat.com> Date: Thu, 15 Aug 2013 11:20:16 +0800 > memcpy_fromiovec is moved to lib/iovec.c. No need to include > linux/socket.h for it. > > Signed-off-by: Asias He <asias@redhat.com> You can't do this. Because this file doesn't include the header file that provides the declaration, which is linux/uio.h linux/socket.h includes linux/uio.h, so honestly leaving things the way they are is a 1000 times better than your patch. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 15, 2013 at 02:07:40PM -0700, David Miller wrote: > From: Asias He <asias@redhat.com> > Date: Thu, 15 Aug 2013 11:20:16 +0800 > > > memcpy_fromiovec is moved to lib/iovec.c. No need to include > > linux/socket.h for it. > > > > Signed-off-by: Asias He <asias@redhat.com> > > You can't do this. > > Because this file doesn't include the header file that > provides the declaration, which is linux/uio.h vhost.c includes drivers/vhost/vhost.h. In drivers/vhost/vhost.h, we have linux/uio.h included. > linux/socket.h includes linux/uio.h, so honestly leaving > things the way they are is a 1000 times better than your > patch. Vhost is a separate module and a generic infrastructure which is not bound to network anymore. I guess it's better to include the real one instead of the socket one.
From: Asias He <asias@redhat.com> Date: Fri, 16 Aug 2013 09:27:43 +0800 > On Thu, Aug 15, 2013 at 02:07:40PM -0700, David Miller wrote: >> From: Asias He <asias@redhat.com> >> Date: Thu, 15 Aug 2013 11:20:16 +0800 >> >> > memcpy_fromiovec is moved to lib/iovec.c. No need to include >> > linux/socket.h for it. >> > >> > Signed-off-by: Asias He <asias@redhat.com> >> >> You can't do this. >> >> Because this file doesn't include the header file that >> provides the declaration, which is linux/uio.h > > vhost.c includes drivers/vhost/vhost.h. In drivers/vhost/vhost.h, we > have linux/uio.h included. Nothing in vhost.h needs linux/uio.h right? That's very poor style, include the header where the dependency exists which is vhost.c -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 16, 2013 at 12:31:59AM -0700, David Miller wrote: > From: Asias He <asias@redhat.com> > Date: Fri, 16 Aug 2013 09:27:43 +0800 > > > On Thu, Aug 15, 2013 at 02:07:40PM -0700, David Miller wrote: > >> From: Asias He <asias@redhat.com> > >> Date: Thu, 15 Aug 2013 11:20:16 +0800 > >> > >> > memcpy_fromiovec is moved to lib/iovec.c. No need to include > >> > linux/socket.h for it. > >> > > >> > Signed-off-by: Asias He <asias@redhat.com> > >> > >> You can't do this. > >> > >> Because this file doesn't include the header file that > >> provides the declaration, which is linux/uio.h > > > > vhost.c includes drivers/vhost/vhost.h. In drivers/vhost/vhost.h, we > > have linux/uio.h included. > > Nothing in vhost.h needs linux/uio.h right? That's very poor style, > include the header where the dependency exists which is vhost.c We use 'struct iovec' in vhost.h which needs linux/uio.h, no? So, how about including linux/uio.h in both vhost.c and vhost.h.
From: Asias He <asias@redhat.com> Date: Fri, 16 Aug 2013 17:27:43 +0800 > On Fri, Aug 16, 2013 at 12:31:59AM -0700, David Miller wrote: >> From: Asias He <asias@redhat.com> >> Date: Fri, 16 Aug 2013 09:27:43 +0800 >> >> > On Thu, Aug 15, 2013 at 02:07:40PM -0700, David Miller wrote: >> >> From: Asias He <asias@redhat.com> >> >> Date: Thu, 15 Aug 2013 11:20:16 +0800 >> >> >> >> > memcpy_fromiovec is moved to lib/iovec.c. No need to include >> >> > linux/socket.h for it. >> >> > >> >> > Signed-off-by: Asias He <asias@redhat.com> >> >> >> >> You can't do this. >> >> >> >> Because this file doesn't include the header file that >> >> provides the declaration, which is linux/uio.h >> > >> > vhost.c includes drivers/vhost/vhost.h. In drivers/vhost/vhost.h, we >> > have linux/uio.h included. >> >> Nothing in vhost.h needs linux/uio.h right? That's very poor style, >> include the header where the dependency exists which is vhost.c > > We use 'struct iovec' in vhost.h which needs linux/uio.h, no? > > So, how about including linux/uio.h in both vhost.c and vhost.h. That sounds good. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index e58cf00..038c242 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -13,7 +13,6 @@ #include <linux/eventfd.h> #include <linux/vhost.h> -#include <linux/socket.h> /* memcpy_fromiovec */ #include <linux/mm.h> #include <linux/mmu_context.h> #include <linux/miscdevice.h>
memcpy_fromiovec is moved to lib/iovec.c. No need to include linux/socket.h for it. Signed-off-by: Asias He <asias@redhat.com> --- drivers/vhost/vhost.c | 1 - 1 file changed, 1 deletion(-)