* [PATCH] vhost: Drop linux/socket.h
@ 2013-08-15 3:20 Asias He
2013-08-15 21:07 ` David Miller
0 siblings, 1 reply; 10+ messages in thread
From: Asias He @ 2013-08-15 3:20 UTC (permalink / raw)
To: netdev; +Cc: virtualization, kvm, Michael S. Tsirkin
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(-)
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>
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] vhost: Drop linux/socket.h
2013-08-15 3:20 [PATCH] vhost: Drop linux/socket.h Asias He
@ 2013-08-15 21:07 ` David Miller
2013-08-16 1:27 ` Asias He
0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2013-08-15 21:07 UTC (permalink / raw)
To: asias; +Cc: netdev, virtualization, kvm, mst
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] vhost: Drop linux/socket.h
2013-08-15 21:07 ` David Miller
@ 2013-08-16 1:27 ` Asias He
2013-08-16 7:31 ` David Miller
0 siblings, 1 reply; 10+ messages in thread
From: Asias He @ 2013-08-16 1:27 UTC (permalink / raw)
To: David Miller; +Cc: netdev, virtualization, kvm, mst
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.
--
Asias
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] vhost: Drop linux/socket.h
2013-08-16 1:27 ` Asias He
@ 2013-08-16 7:31 ` David Miller
2013-08-16 9:27 ` Asias He
2013-08-16 9:50 ` [PATCH] vhost: Drop linux/socket.h Michael S. Tsirkin
0 siblings, 2 replies; 10+ messages in thread
From: David Miller @ 2013-08-16 7:31 UTC (permalink / raw)
To: asias; +Cc: netdev, virtualization, kvm, mst
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] vhost: Drop linux/socket.h
2013-08-16 7:31 ` David Miller
@ 2013-08-16 9:27 ` Asias He
2013-08-17 5:29 ` David Miller
2013-08-16 9:50 ` [PATCH] vhost: Drop linux/socket.h Michael S. Tsirkin
1 sibling, 1 reply; 10+ messages in thread
From: Asias He @ 2013-08-16 9:27 UTC (permalink / raw)
To: David Miller; +Cc: netdev, virtualization, kvm, mst
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.
--
Asias
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] vhost: Drop linux/socket.h
2013-08-16 7:31 ` David Miller
2013-08-16 9:27 ` Asias He
@ 2013-08-16 9:50 ` Michael S. Tsirkin
1 sibling, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2013-08-16 9:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev, kvm, virtualization
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
It needs struct iovec, which is in include/uapi/linux/uio.h.
Do you think it's better to include uapi/linux/uio.h directly?
In that case maybe we should update linux/socket.h
to pull in uapi/linux/uio.h directly too.
Or even just forward-declare struct iovec.
I tried this last option and the only fall-out seems to be in security/
- though I didn't do an allyes config build yet. Good idea? If yes let
me know and I'll do that.
-->
socket: forward-declare struct iovec
We can simplify header dependencies by using a forward declaration
of struct iovec in socket.h - it's always used through a pointer there.
Do this and fix up users that rely on socket.h including uio.h.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 230c04b..3ad6f52 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -4,7 +4,6 @@
#include <asm/socket.h> /* arch-dependent defines */
#include <linux/sockios.h> /* the SIOCxxx I/O controls */
-#include <linux/uio.h> /* iovec support */
#include <linux/types.h> /* pid_t */
#include <linux/compiler.h> /* __user */
#include <uapi/linux/socket.h>
@@ -44,6 +43,8 @@ struct linger {
* belong in an obscure libc emulation or the bin.
*/
+struct iovec;
+
struct msghdr {
void * msg_name; /* Socket name */
int msg_namelen; /* Length of name */
diff --git a/security/keys/compat.c b/security/keys/compat.c
index d65fa7f..9c2723f 100644
--- a/security/keys/compat.c
+++ b/security/keys/compat.c
@@ -13,6 +13,7 @@
#include <linux/keyctl.h>
#include <linux/compat.h>
#include <linux/slab.h>
+#include <uapi/linux/uio.h>
#include "internal.h"
/*
--
MST
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] vhost: Drop linux/socket.h
2013-08-16 9:27 ` Asias He
@ 2013-08-17 5:29 ` David Miller
2013-08-19 1:23 ` [PATCH v2] vhost: Include linux/uio.h instead of linux/socket.h Asias He
0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2013-08-17 5:29 UTC (permalink / raw)
To: asias; +Cc: netdev, virtualization, kvm, mst
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] vhost: Include linux/uio.h instead of linux/socket.h
2013-08-17 5:29 ` David Miller
@ 2013-08-19 1:23 ` Asias He
2013-08-19 7:24 ` Michael S. Tsirkin
2013-08-20 22:08 ` David Miller
0 siblings, 2 replies; 10+ messages in thread
From: Asias He @ 2013-08-19 1:23 UTC (permalink / raw)
To: netdev; +Cc: kvm, Michael S. Tsirkin, virtualization, David Miller
memcpy_fromiovec is moved from net/core/iovec.c to lib/iovec.c.
linux/uio.h provides the declaration for memcpy_fromiovec.
Include linux/uio.h instead of inux/socket.h for it.
Signed-off-by: Asias He <asias@redhat.com>
---
drivers/vhost/vhost.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index e58cf00..448efe0 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -13,7 +13,7 @@
#include <linux/eventfd.h>
#include <linux/vhost.h>
-#include <linux/socket.h> /* memcpy_fromiovec */
+#include <linux/uio.h>
#include <linux/mm.h>
#include <linux/mmu_context.h>
#include <linux/miscdevice.h>
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] vhost: Include linux/uio.h instead of linux/socket.h
2013-08-19 1:23 ` [PATCH v2] vhost: Include linux/uio.h instead of linux/socket.h Asias He
@ 2013-08-19 7:24 ` Michael S. Tsirkin
2013-08-20 22:08 ` David Miller
1 sibling, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2013-08-19 7:24 UTC (permalink / raw)
To: Asias He; +Cc: netdev, David Miller, kvm, virtualization
On Mon, Aug 19, 2013 at 09:23:19AM +0800, Asias He wrote:
> memcpy_fromiovec is moved from net/core/iovec.c to lib/iovec.c.
> linux/uio.h provides the declaration for memcpy_fromiovec.
>
> Include linux/uio.h instead of inux/socket.h for it.
>
> Signed-off-by: Asias He <asias@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/vhost/vhost.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index e58cf00..448efe0 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -13,7 +13,7 @@
>
> #include <linux/eventfd.h>
> #include <linux/vhost.h>
> -#include <linux/socket.h> /* memcpy_fromiovec */
> +#include <linux/uio.h>
> #include <linux/mm.h>
> #include <linux/mmu_context.h>
> #include <linux/miscdevice.h>
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] vhost: Include linux/uio.h instead of linux/socket.h
2013-08-19 1:23 ` [PATCH v2] vhost: Include linux/uio.h instead of linux/socket.h Asias He
2013-08-19 7:24 ` Michael S. Tsirkin
@ 2013-08-20 22:08 ` David Miller
1 sibling, 0 replies; 10+ messages in thread
From: David Miller @ 2013-08-20 22:08 UTC (permalink / raw)
To: asias; +Cc: netdev, virtualization, kvm, mst
From: Asias He <asias@redhat.com>
Date: Mon, 19 Aug 2013 09:23:19 +0800
> memcpy_fromiovec is moved from net/core/iovec.c to lib/iovec.c.
> linux/uio.h provides the declaration for memcpy_fromiovec.
>
> Include linux/uio.h instead of inux/socket.h for it.
>
> Signed-off-by: Asias He <asias@redhat.com>
Applied.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-08-20 22:08 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-15 3:20 [PATCH] vhost: Drop linux/socket.h Asias He
2013-08-15 21:07 ` David Miller
2013-08-16 1:27 ` Asias He
2013-08-16 7:31 ` David Miller
2013-08-16 9:27 ` Asias He
2013-08-17 5:29 ` David Miller
2013-08-19 1:23 ` [PATCH v2] vhost: Include linux/uio.h instead of linux/socket.h Asias He
2013-08-19 7:24 ` Michael S. Tsirkin
2013-08-20 22:08 ` David Miller
2013-08-16 9:50 ` [PATCH] vhost: Drop linux/socket.h Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.