Cleanup num_found usage in gdb_wait_for_event
Commit Message
Probably an artifact from the past, managing num_found in those loops is
not need.
Regtested on Ubuntu 14.04 x64, although only with use_poll == 1.
gdb/ChangeLog:
* event-loop.c (gdb_wait_for_event): Cleanup uneeded usages of
num_found.
---
gdb/event-loop.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
Comments
On 04/29/2015 07:19 PM, Simon Marchi wrote:
> Probably an artifact from the past, managing num_found in those loops is
> not need.
>
> Regtested on Ubuntu 14.04 x64, although only with use_poll == 1.
>
> gdb/ChangeLog:
>
> * event-loop.c (gdb_wait_for_event): Cleanup uneeded usages of
> num_found.
Hmm, I'm not sure whether the state of poll_fds[i].revents is defined
if poll returns timeout or -1/EINTR, and it seems that with this change
we'll walk the poll_fds array on -1/EINTR.
I was planning on sending this patch soon that touches this code, and
makes use of num_found:
https://github.com/palves/gdb/commit/5652cc4487a1cc5e1d5ab85b64f325292fd059f2
Now I'm curious how you ran into this.
Thanks,
Pedro Alves
On 15-04-29 03:19 PM, Pedro Alves wrote:
> On 04/29/2015 07:19 PM, Simon Marchi wrote:
>> Probably an artifact from the past, managing num_found in those loops is
>> not need.
>>
>> Regtested on Ubuntu 14.04 x64, although only with use_poll == 1.
>>
>> gdb/ChangeLog:
>>
>> * event-loop.c (gdb_wait_for_event): Cleanup uneeded usages of
>> num_found.
>
> Hmm, I'm not sure whether the state of poll_fds[i].revents is defined
> if poll returns timeout or -1/EINTR, and it seems that with this change
> we'll walk the poll_fds array on -1/EINTR.
>
> I was planning on sending this patch soon that touches this code, and
> makes use of num_found:
>
> https://github.com/palves/gdb/commit/5652cc4487a1cc5e1d5ab85b64f325292fd059f2
>
> Now I'm curious how you ran into this.
>
> Thanks,
> Pedro Alves
Oh well, I was investigating the exact same test failure (mi-nsmoribund.exp). It's
failing maybe 90% of the time on our x86 Jenkins boxes. I was actually writing a
patch for that at the moment.
For poll, I keep a static variable around that tells which pollfd structure to start
iterating from. So the first time we check 0, 1, 2, next time we'll check 1, 2, 0, then
2, 0, 1, etc. It's really simple. The static variable should be a field in gdb_notifier
though.
For select, I move the handled file_ptr at the end of the linked list, so that it has the
least priority the next time around. You solution to keep a pointer to the first file_ptr
to check next time is simpler though. Then you can just iterate on the list starting at
that item, making sure you handle correctly the wrap at the end of the list.
Here is my patch in raw form:
https://github.com/simark/binutils-gdb/commit/ec49aaec6963f0aa974d183b8fca669f6261274d
In any case, I don't see why interacting with num_found here is useful, given that we only
handle one event. If we did handle all the found events, then I would understand, since you
want to know when you are done.
@@ -751,24 +751,22 @@ gdb_wait_for_event (int block)
if (use_poll)
{
#ifdef HAVE_POLL
- for (i = 0; (i < gdb_notifier.num_fds) && (num_found > 0); i++)
+ for (i = 0; i < gdb_notifier.num_fds; i++)
{
- if ((gdb_notifier.poll_fds + i)->revents)
- num_found--;
- else
+ if (!gdb_notifier.poll_fds[i].revents)
continue;
for (file_ptr = gdb_notifier.first_file_handler;
file_ptr != NULL;
file_ptr = file_ptr->next_file)
{
- if (file_ptr->fd == (gdb_notifier.poll_fds + i)->fd)
+ if (file_ptr->fd == gdb_notifier.poll_fds[i].fd)
break;
}
if (file_ptr)
{
- int mask = (gdb_notifier.poll_fds + i)->revents;
+ int mask = gdb_notifier.poll_fds[i].revents;
handle_file_event (file_ptr, mask);
return 1;
@@ -782,7 +780,7 @@ gdb_wait_for_event (int block)
else
{
for (file_ptr = gdb_notifier.first_file_handler;
- (file_ptr != NULL) && (num_found > 0);
+ file_ptr != NULL;
file_ptr = file_ptr->next_file)
{
int mask = 0;
@@ -796,8 +794,6 @@ gdb_wait_for_event (int block)
if (!mask)
continue;
- else
- num_found--;
handle_file_event (file_ptr, mask);
return 1;