Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to include/openvswitch

[Xiao Liang]

On Fri, Nov 3, 2017 at 1:12 AM, Ben Pfaff <b...@ovn.org> wrote:
> On Thu, Nov 02, 2017 at 01:35:11PM +, Alin Serdean wrote:
>>
>>
>> > -Original Message-
>> > From: Xiao Liang [mailto:shaw.l...@gmail.com]
>> > Sent: Thursday, November 2, 2017 4:32 AM
>> > To: Alin Serdean <aserd...@cloudbasesolutions.com>
>> > Cc: Ben Pfaff <b...@ovn.org>; d...@openvswitch.org
>> > Subject: Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to
>> > include/openvswitch
>> >
>> > On Thu, Nov 2, 2017 at 5:51 AM, Alin Serdean
>> > <aserd...@cloudbasesolutions.com> wrote:
>> > >
>> > >
>> > >> -Original Message-
>> > >> From: Ben Pfaff [mailto:b...@ovn.org]
>> > >> Sent: Tuesday, October 31, 2017 9:34 PM
>> > >> To: Xiao Liang <shaw.l...@gmail.com>; Alin Serdean
>> > >> <aserd...@cloudbasesolutions.com>
>> > >> Cc: d...@openvswitch.org
>> > >> Subject: Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to
>> > >> include/openvswitch
>> > >>
>> > >> On Thu, Aug 17, 2017 at 12:06:25AM +0800, Xiao Liang wrote:
>> > >> > Poll-loop is the core to implement main loop. It should be
>> > >> > available in libopenvswitch.
>> > >> >
>> > >> > Signed-off-by: Xiao Liang <shaw.l...@gmail.com>
>> > >>
>> > >> I'm concerned about the way that this adds a definition of HANDLE in
>> > >> a public header.  That seems unfriendly to code that might want to
>> > >> include both this header and Win32 headers that properly define HANDLE.
>> > >>
>> > >> Alin, what's the right thing to do here?
>> > > [Alin Serdean] First the type definition is wrong (HANDLE is VOID*).
>> > > I would avoid adding the definition to HANDLE. Maybe add an include to
>> >  or  to include/windows/poll.h .
>> >
>> > Since include/windows is not installed as public header, is it safe to 
>> > remove
>> > inclusion of poll.h and add windows.h in poll-loop.h?
>> That sounds reasonable to me. Ben do you agree?
>
>  is definitely needed on non-Windows platforms for the
> definition of POLLIN and POLLOUT, so it would have to be conditional.  I
> don't object to including , on Windows, but I don't know the
> full implications of doing it.

Thanks. I submitted a new patch.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to include/openvswitch

[Ben Pfaff]

On Thu, Nov 02, 2017 at 01:35:11PM +, Alin Serdean wrote:
> 
> 
> > -Original Message-
> > From: Xiao Liang [mailto:shaw.l...@gmail.com]
> > Sent: Thursday, November 2, 2017 4:32 AM
> > To: Alin Serdean <aserd...@cloudbasesolutions.com>
> > Cc: Ben Pfaff <b...@ovn.org>; d...@openvswitch.org
> > Subject: Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to
> > include/openvswitch
> > 
> > On Thu, Nov 2, 2017 at 5:51 AM, Alin Serdean
> > <aserd...@cloudbasesolutions.com> wrote:
> > >
> > >
> > >> -Original Message-
> > >> From: Ben Pfaff [mailto:b...@ovn.org]
> > >> Sent: Tuesday, October 31, 2017 9:34 PM
> > >> To: Xiao Liang <shaw.l...@gmail.com>; Alin Serdean
> > >> <aserd...@cloudbasesolutions.com>
> > >> Cc: d...@openvswitch.org
> > >> Subject: Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to
> > >> include/openvswitch
> > >>
> > >> On Thu, Aug 17, 2017 at 12:06:25AM +0800, Xiao Liang wrote:
> > >> > Poll-loop is the core to implement main loop. It should be
> > >> > available in libopenvswitch.
> > >> >
> > >> > Signed-off-by: Xiao Liang <shaw.l...@gmail.com>
> > >>
> > >> I'm concerned about the way that this adds a definition of HANDLE in
> > >> a public header.  That seems unfriendly to code that might want to
> > >> include both this header and Win32 headers that properly define HANDLE.
> > >>
> > >> Alin, what's the right thing to do here?
> > > [Alin Serdean] First the type definition is wrong (HANDLE is VOID*).
> > > I would avoid adding the definition to HANDLE. Maybe add an include to
> >  or  to include/windows/poll.h .
> > 
> > Since include/windows is not installed as public header, is it safe to 
> > remove
> > inclusion of poll.h and add windows.h in poll-loop.h?
> That sounds reasonable to me. Ben do you agree?

 is definitely needed on non-Windows platforms for the
definition of POLLIN and POLLOUT, so it would have to be conditional.  I
don't object to including , on Windows, but I don't know the
full implications of doing it.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to include/openvswitch

[Alin Serdean]

> -Original Message-
> From: Xiao Liang [mailto:shaw.l...@gmail.com]
> Sent: Thursday, November 2, 2017 4:32 AM
> To: Alin Serdean <aserd...@cloudbasesolutions.com>
> Cc: Ben Pfaff <b...@ovn.org>; d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to
> include/openvswitch
> 
> On Thu, Nov 2, 2017 at 5:51 AM, Alin Serdean
> <aserd...@cloudbasesolutions.com> wrote:
> >
> >
> >> -Original Message-
> >> From: Ben Pfaff [mailto:b...@ovn.org]
> >> Sent: Tuesday, October 31, 2017 9:34 PM
> >> To: Xiao Liang <shaw.l...@gmail.com>; Alin Serdean
> >> <aserd...@cloudbasesolutions.com>
> >> Cc: d...@openvswitch.org
> >> Subject: Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to
> >> include/openvswitch
> >>
> >> On Thu, Aug 17, 2017 at 12:06:25AM +0800, Xiao Liang wrote:
> >> > Poll-loop is the core to implement main loop. It should be
> >> > available in libopenvswitch.
> >> >
> >> > Signed-off-by: Xiao Liang <shaw.l...@gmail.com>
> >>
> >> I'm concerned about the way that this adds a definition of HANDLE in
> >> a public header.  That seems unfriendly to code that might want to
> >> include both this header and Win32 headers that properly define HANDLE.
> >>
> >> Alin, what's the right thing to do here?
> > [Alin Serdean] First the type definition is wrong (HANDLE is VOID*).
> > I would avoid adding the definition to HANDLE. Maybe add an include to
>  or  to include/windows/poll.h .
> 
> Since include/windows is not installed as public header, is it safe to remove
> inclusion of poll.h and add windows.h in poll-loop.h?
That sounds reasonable to me. Ben do you agree?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to include/openvswitch

[Xiao Liang]

On Thu, Nov 2, 2017 at 5:51 AM, Alin Serdean
<aserd...@cloudbasesolutions.com> wrote:
>
>
>> -Original Message-
>> From: Ben Pfaff [mailto:b...@ovn.org]
>> Sent: Tuesday, October 31, 2017 9:34 PM
>> To: Xiao Liang <shaw.l...@gmail.com>; Alin Serdean
>> <aserd...@cloudbasesolutions.com>
>> Cc: d...@openvswitch.org
>> Subject: Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to
>> include/openvswitch
>>
>> On Thu, Aug 17, 2017 at 12:06:25AM +0800, Xiao Liang wrote:
>> > Poll-loop is the core to implement main loop. It should be available
>> > in libopenvswitch.
>> >
>> > Signed-off-by: Xiao Liang <shaw.l...@gmail.com>
>>
>> I'm concerned about the way that this adds a definition of HANDLE in a public
>> header.  That seems unfriendly to code that might want to include both this
>> header and Win32 headers that properly define HANDLE.
>>
>> Alin, what's the right thing to do here?
> [Alin Serdean] First the type definition is wrong (HANDLE is VOID*).
> I would avoid adding the definition to HANDLE. Maybe add an include to 
>  or  to include/windows/poll.h .

Since include/windows is not installed as public header, is it safe to
remove inclusion of poll.h and add windows.h in poll-loop.h?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to include/openvswitch

[Alin Serdean]

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Tuesday, October 31, 2017 9:34 PM
> To: Xiao Liang <shaw.l...@gmail.com>; Alin Serdean
> <aserd...@cloudbasesolutions.com>
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to
> include/openvswitch
> 
> On Thu, Aug 17, 2017 at 12:06:25AM +0800, Xiao Liang wrote:
> > Poll-loop is the core to implement main loop. It should be available
> > in libopenvswitch.
> >
> > Signed-off-by: Xiao Liang <shaw.l...@gmail.com>
> 
> I'm concerned about the way that this adds a definition of HANDLE in a public
> header.  That seems unfriendly to code that might want to include both this
> header and Win32 headers that properly define HANDLE.
> 
> Alin, what's the right thing to do here?
[Alin Serdean] First the type definition is wrong (HANDLE is VOID*).
I would avoid adding the definition to HANDLE. Maybe add an include to 
 or  to include/windows/poll.h .
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to include/openvswitch

[Ben Pfaff]

On Thu, Aug 17, 2017 at 12:06:25AM +0800, Xiao Liang wrote:
> Poll-loop is the core to implement main loop. It should be available in
> libopenvswitch.
> 
> Signed-off-by: Xiao Liang 

I'm concerned about the way that this adds a definition of HANDLE in a
public header.  That seems unfriendly to code that might want to include
both this header and Win32 headers that properly define HANDLE.

Alin, what's the right thing to do here?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to include/openvswitch

[Xiao Liang]

Poll-loop is the core to implement main loop. It should be available in
libopenvswitch.

Signed-off-by: Xiao Liang 
---
 include/openvswitch/automake.mk   | 1 +
 {lib => include/openvswitch}/poll-loop.h  | 2 +-
 lib/bfd.c | 2 +-
 lib/cfm.c | 2 +-
 lib/conntrack.c   | 2 +-
 lib/daemon-windows.c  | 2 +-
 lib/dpif-netdev.c | 2 +-
 lib/dpif-netlink.c| 2 +-
 lib/dpif.c| 2 +-
 lib/fatal-signal.c| 2 +-
 lib/jsonrpc.c | 2 +-
 lib/lacp.c| 2 +-
 lib/latch-unix.c  | 2 +-
 lib/latch-windows.c   | 2 +-
 lib/learning-switch.c | 2 +-
 lib/mac-learning.c| 2 +-
 lib/mcast-snooping.c  | 2 +-
 lib/memory.c  | 2 +-
 lib/netdev-bsd.c  | 2 +-
 lib/netdev-dummy.c| 2 +-
 lib/netdev-linux.c| 2 +-
 lib/netdev-vport.c| 2 +-
 lib/netdev-windows.c  | 2 +-
 lib/netdev.c  | 2 +-
 lib/netlink-conntrack.c   | 2 +-
 lib/netlink-socket.c  | 2 +-
 lib/ovs-lldp.c| 2 +-
 lib/ovs-rcu.c | 2 +-
 lib/ovs-thread.c  | 2 +-
 lib/ovsdb-idl.c   | 2 +-
 lib/poll-loop.c   | 2 +-
 lib/process.c | 2 +-
 lib/rconn.c   | 2 +-
 lib/reconnect.c   | 2 +-
 lib/rtbsd.c   | 2 +-
 lib/seq.c | 2 +-
 lib/signals.c | 2 +-
 lib/socket-util.c | 2 +-
 lib/stream-fd.c   | 2 +-
 lib/stream-ssl.c  | 2 +-
 lib/stream-unix.c | 2 +-
 lib/stream-windows.c  | 2 +-
 lib/stream.c  | 2 +-
 lib/timer.c   | 2 +-
 lib/tnl-neigh-cache.c | 2 +-
 lib/token-bucket.c| 2 +-
 lib/unixctl.c | 2 +-
 lib/vconn-stream.c| 2 +-
 lib/vconn.c   | 2 +-
 ofproto/bond.c| 2 +-
 ofproto/bundles.c | 2 +-
 ofproto/connmgr.c | 2 +-
 ofproto/fail-open.c   | 2 +-
 ofproto/in-band.c | 2 +-
 ofproto/netflow.c | 2 +-
 ofproto/ofproto-dpif-ipfix.c  | 2 +-
 ofproto/ofproto-dpif-monitor.c| 2 +-
 ofproto/ofproto-dpif-sflow.c  | 2 +-
 ofproto/ofproto-dpif-upcall.c | 2 +-
 ofproto/ofproto-dpif.c| 2 +-
 ofproto/ofproto.c | 2 +-
 ofproto/pinsched.c| 2 +-
 ovn/controller-vtep/gateway.c | 2 +-
 ovn/controller-vtep/ovn-controller-vtep.c | 2 +-
 ovn/controller/binding.c  | 2 +-
 ovn/controller/ofctrl.c   | 2 +-
 ovn/controller/ovn-controller.c   | 2 +-
 ovn/controller/physical.c | 2 +-
 ovn/controller/pinctrl.c  | 2 +-
 ovn/northd/ovn-northd.c   | 2 +-
 ovn/utilities/ovn-nbctl.c | 2 +-
 ovn/utilities/ovn-sbctl.c | 2 +-
 ovn/utilities/ovn-trace.c | 2 +-
 ovsdb/jsonrpc-server.c| 2 +-
 ovsdb/ovsdb-client.c  | 2 +-
 ovsdb/ovsdb-server.c  | 2 +-
 ovsdb/trigger.c   | 3 ++-
 tests/test-jsonrpc.c  | 2 +-
 tests/test-netflow.c  | 2 +-
 tests/test-netlink-conntrack.c| 2 +-
 tests/test-ovsdb.c| 2 +-
 tests/test-sflow.c| 2 +-
 tests/test-unixctl.c  | 2 +-
 tests/test-vconn.c| 2 +-
 utilities/nlmon.c | 2 +-
 utilities/ovs-ofctl.c | 2 +-
 utilities/ovs-testcontroller.c| 2 +-
 utilities/ovs-vsctl.c | 2 +-
 vswitchd/bridge.c | 2 +-
 vswitchd/ovs-vswitchd.c   | 2 +-
 vswitchd/system-stats.c   | 2 +-
 vtep/vtep-ctl.c   | 2 +-
 92 files changed, 93 insertions(+), 91 deletions(-)
 rename {lib => include/openvswitch}/poll-loop.h (99%)

diff --git a/include/openvswitch/automake.mk b/include/openvswitch/automake.mk
index cb0080d53..392dcf983 100644
---