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

Xiao Liang shaw.leon at gmail.com
Fri Nov 3 05:56:43 UTC 2017


On Fri, Nov 3, 2017 at 1:12 AM, Ben Pfaff <blp at ovn.org> wrote:
> On Thu, Nov 02, 2017 at 01:35:11PM +0000, Alin Serdean wrote:
>>
>>
>> > -----Original Message-----
>> > From: Xiao Liang [mailto:shaw.leon at gmail.com]
>> > Sent: Thursday, November 2, 2017 4:32 AM
>> > To: Alin Serdean <aserdean at cloudbasesolutions.com>
>> > Cc: Ben Pfaff <blp at ovn.org>; dev at 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
>> > <aserdean at cloudbasesolutions.com> wrote:
>> > >
>> > >
>> > >> -----Original Message-----
>> > >> From: Ben Pfaff [mailto:blp at ovn.org]
>> > >> Sent: Tuesday, October 31, 2017 9:34 PM
>> > >> To: Xiao Liang <shaw.leon at gmail.com>; Alin Serdean
>> > >> <aserdean at cloudbasesolutions.com>
>> > >> Cc: dev at 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.leon at 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
>> > <windows.h> or <windefs.h> 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?
>
> <poll.h> 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 <windows.h>, on Windows, but I don't know the
> full implications of doing it.

Thanks. I submitted a new patch.


More information about the dev mailing list