[ovs-dev] poll-loop: fix assertion in poll_create_node
diff mbox

Message ID 1442914035-7869-1-git-send-email-i.maximets@samsung.com
State Accepted
Headers show

Commit Message

Ilya Maximets Sept. 22, 2015, 9:27 a.m. UTC
Zero is a valid value for a file descriptor.

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Signed-off-by: Nikita Kalyazin <n.kalyazin@samsung.com>
---
 lib/poll-loop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Ben Pfaff Sept. 22, 2015, 3:49 p.m. UTC | #1
On Tue, Sep 22, 2015 at 12:27:15PM +0300, Ilya Maximets wrote:
> Zero is a valid value for a file descriptor.
> 
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> Signed-off-by: Nikita Kalyazin <n.kalyazin@samsung.com>

Thanks for the patch!

The sign-off chain is unclear.  Who is the author?  What did the other
person do?

Thanks,

Ben.
Ilya Maximets Sept. 22, 2015, 4:30 p.m. UTC | #2
Author is me, Nikita found and analyzed the problem.
You may change Signed-off-by to something more proper.

Best regards, Ilya Maximets.

On 22.09.2015 18:49, Ben Pfaff wrote:
> On Tue, Sep 22, 2015 at 12:27:15PM +0300, Ilya Maximets wrote:
>> Zero is a valid value for a file descriptor.
>>
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>> Signed-off-by: Nikita Kalyazin <n.kalyazin@samsung.com>
> 
> Thanks for the patch!
> 
> The sign-off chain is unclear.  Who is the author?  What did the other
> person do?
> 
> Thanks,
> 
> Ben.
>
Ben Pfaff Sept. 22, 2015, 4:34 p.m. UTC | #3
Thanks, I understand now.  I applied this to master, changing Nikita's
Signed-off-by: to a Reported-by:.

On Tue, Sep 22, 2015 at 07:30:47PM +0300, Ilya Maximets wrote:
> Author is me, Nikita found and analyzed the problem.
> You may change Signed-off-by to something more proper.
> 
> Best regards, Ilya Maximets.
> 
> On 22.09.2015 18:49, Ben Pfaff wrote:
> > On Tue, Sep 22, 2015 at 12:27:15PM +0300, Ilya Maximets wrote:
> >> Zero is a valid value for a file descriptor.
> >>
> >> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> >> Signed-off-by: Nikita Kalyazin <n.kalyazin@samsung.com>
> > 
> > Thanks for the patch!
> > 
> > The sign-off chain is unclear.  Who is the author?  What did the other
> > person do?
> > 
> > Thanks,
> > 
> > Ben.
> >
Gurucharan Shetty Sept. 29, 2015, 9:39 p.m. UTC | #4
Windows even handling selector functions is such that it does not like
when fd is -1 (I have not root-caused the exact reason). This commit
causes most of the unit tests on Windows to fail. So the larger
question is, even though fd of zero is valid, do we really pass it to
any of our poll node functions?

On Tue, Sep 22, 2015 at 9:34 AM, Ben Pfaff <blp@nicira.com> wrote:
> Thanks, I understand now.  I applied this to master, changing Nikita's
> Signed-off-by: to a Reported-by:.
>
> On Tue, Sep 22, 2015 at 07:30:47PM +0300, Ilya Maximets wrote:
>> Author is me, Nikita found and analyzed the problem.
>> You may change Signed-off-by to something more proper.
>>
>> Best regards, Ilya Maximets.
>>
>> On 22.09.2015 18:49, Ben Pfaff wrote:
>> > On Tue, Sep 22, 2015 at 12:27:15PM +0300, Ilya Maximets wrote:
>> >> Zero is a valid value for a file descriptor.
>> >>
>> >> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>> >> Signed-off-by: Nikita Kalyazin <n.kalyazin@samsung.com>
>> >
>> > Thanks for the patch!
>> >
>> > The sign-off chain is unclear.  Who is the author?  What did the other
>> > person do?
>> >
>> > Thanks,
>> >
>> > Ben.
>> >
Alin Serdean Sept. 29, 2015, 9:48 p.m. UTC | #5
I am trying to find the root cause also Guru, it seems that someone is passing an invalid handle.

Alin.

> -----Mesaj original-----

> De la: dev [mailto:dev-bounces@openvswitch.org] În numele Gurucharan

> Shetty

> Trimis: Wednesday, September 30, 2015 12:40 AM

> Către: Ben Pfaff <blp@nicira.com>

> Cc: Ilya Maximets <i.maximets@samsung.com>; dev

> <dev@openvswitch.org>; Dyasly Sergey <s.dyasly@samsung.com>

> Subiect: Re: [ovs-dev] [PATCH] poll-loop: fix assertion in poll_create_node

> 

> Windows even handling selector functions is such that it does not like when

> fd is -1 (I have not root-caused the exact reason). This commit causes most of

> the unit tests on Windows to fail. So the larger question is, even though fd of

> zero is valid, do we really pass it to any of our poll node functions?

> 

> On Tue, Sep 22, 2015 at 9:34 AM, Ben Pfaff <blp@nicira.com> wrote:

> > Thanks, I understand now.  I applied this to master, changing Nikita's

> > Signed-off-by: to a Reported-by:.

> >

> > On Tue, Sep 22, 2015 at 07:30:47PM +0300, Ilya Maximets wrote:

> >> Author is me, Nikita found and analyzed the problem.

> >> You may change Signed-off-by to something more proper.

> >>

> >> Best regards, Ilya Maximets.

> >>

> >> On 22.09.2015 18:49, Ben Pfaff wrote:

> >> > On Tue, Sep 22, 2015 at 12:27:15PM +0300, Ilya Maximets wrote:

> >> >> Zero is a valid value for a file descriptor.

> >> >>

> >> >> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>

> >> >> Signed-off-by: Nikita Kalyazin <n.kalyazin@samsung.com>

> >> >

> >> > Thanks for the patch!

> >> >

> >> > The sign-off chain is unclear.  Who is the author?  What did the

> >> > other person do?

> >> >

> >> > Thanks,

> >> >

> >> > Ben.

> >> >

> _______________________________________________

> dev mailing list

> dev@openvswitch.org

> http://openvswitch.org/mailman/listinfo/dev

Patch
diff mbox

diff --git a/lib/poll-loop.c b/lib/poll-loop.c
index 3c4b55c..36eb5ac 100644
--- a/lib/poll-loop.c
+++ b/lib/poll-loop.c
@@ -104,7 +104,7 @@  poll_create_node(int fd, HANDLE wevent, short int events, const char *where)
     COVERAGE_INC(poll_create_node);
 
     /* Both 'fd' and 'wevent' cannot be set. */
-    ovs_assert(!fd != !wevent);
+    ovs_assert(fd == -1 || !wevent);
 
     /* Check for duplicate.  If found, "or" the events. */
     node = find_poll_node(loop, fd, wevent);
@@ -159,7 +159,7 @@  poll_fd_wait_at(int fd, short int events, const char *where)
 void
 poll_wevent_wait_at(HANDLE wevent, const char *where)
 {
-    poll_create_node(0, wevent, 0, where);
+    poll_create_node(-1, wevent, 0, where);
 }
 #endif /* _WIN32 */