* [PATCH] Fix PR remote/15974
@ 2013-11-30 5:47 Yao Qi
2013-11-30 10:11 ` Pedro Alves
0 siblings, 1 reply; 4+ messages in thread
From: Yao Qi @ 2013-11-30 5:47 UTC (permalink / raw)
To: gdb-patches
In remote-notif.c:handle_notification, we have a loop,
for (i = 0; i < ARRAY_SIZE (notifs); i++)
{
nc = notifs[i];
if (strncmp (buf, nc->name, strlen (nc->name)) == 0
&& buf[strlen (nc->name)] == ':')
break;
}
/* We ignore notifications we don't recognize, for compatibility
with newer stubs. */
if (nc == NULL)
return;
If the notification is not in the list 'notifs', the last entry is
used, which is wrong. It should be NULL. This patch sets 'nc' to
NULL in the loop.
gdb:
2013-11-30 Yao Qi <yao@codesourcery.com>
PR remote/15974
* remote-notif.c (handle_notification): Set variable 'nc' to
NULL in loop.
---
gdb/remote-notif.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
index 0d59279..598e888 100644
--- a/gdb/remote-notif.c
+++ b/gdb/remote-notif.c
@@ -136,6 +136,7 @@ handle_notification (struct remote_notif_state *state, char *buf)
if (strncmp (buf, nc->name, strlen (nc->name)) == 0
&& buf[strlen (nc->name)] == ':')
break;
+ nc = NULL;
}
/* We ignore notifications we don't recognize, for compatibility
--
1.7.7.6
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] Fix PR remote/15974
2013-11-30 5:47 [PATCH] Fix PR remote/15974 Yao Qi
@ 2013-11-30 10:11 ` Pedro Alves
2013-12-02 6:41 ` Yao Qi
0 siblings, 1 reply; 4+ messages in thread
From: Pedro Alves @ 2013-11-30 10:11 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 11/30/2013 03:38 AM, Yao Qi wrote:
> In remote-notif.c:handle_notification, we have a loop,
>
> for (i = 0; i < ARRAY_SIZE (notifs); i++)
> {
> nc = notifs[i];
> if (strncmp (buf, nc->name, strlen (nc->name)) == 0
> && buf[strlen (nc->name)] == ':')
> break;
> }
>
> /* We ignore notifications we don't recognize, for compatibility
> with newer stubs. */
> if (nc == NULL)
> return;
>
> If the notification is not in the list 'notifs', the last entry is
> used, which is wrong. It should be NULL. This patch sets 'nc' to
> NULL in the loop.
>
> gdb:
>
> 2013-11-30 Yao Qi <yao@codesourcery.com>
>
> PR remote/15974
> * remote-notif.c (handle_notification): Set variable 'nc' to
> NULL in loop.
> ---
> gdb/remote-notif.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
> index 0d59279..598e888 100644
> --- a/gdb/remote-notif.c
> +++ b/gdb/remote-notif.c
> @@ -136,6 +136,7 @@ handle_notification (struct remote_notif_state *state, char *buf)
> if (strncmp (buf, nc->name, strlen (nc->name)) == 0
> && buf[strlen (nc->name)] == ':')
> break;
> + nc = NULL;
> }
>
> /* We ignore notifications we don't recognize, for compatibility
>
Instead of setting and unsetting NC at each iteration, the "canonical" way
for this sort of thing is to check whether the loop ended:
- if (nc == NULL)
+ for (i == ARRAY_SIZE (notifs))
And then again, we don't really need NC in the loop. I think
this would look cleaner and clearer:
diff --git c/gdb/remote-notif.c w/gdb/remote-notif.c
index 0d59279..efd1509 100644
--- c/gdb/remote-notif.c
+++ w/gdb/remote-notif.c
@@ -127,22 +127,25 @@ remote_async_get_pending_events_handler (gdb_client_data data)
void
handle_notification (struct remote_notif_state *state, char *buf)
{
- struct notif_client *nc = NULL;
- int i;
+ struct notif_client *nc;
+ size_t i;
for (i = 0; i < ARRAY_SIZE (notifs); i++)
{
- nc = notifs[i];
- if (strncmp (buf, nc->name, strlen (nc->name)) == 0
- && buf[strlen (nc->name)] == ':')
+ const char *name = notifs[i]->name;
+
+ if (strncmp (buf, name, strlen (name)) == 0
+ && buf[strlen (name)] == ':')
break;
}
/* We ignore notifications we don't recognize, for compatibility
with newer stubs. */
- if (nc == NULL)
+ if (i == ARRAY_SIZE (notifs))
return;
+ nc = notifs[i];
+
if (state->pending_event[nc->id] != NULL)
{
/* We've already parsed the in-flight reply, but the stub for some
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] Fix PR remote/15974
2013-11-30 10:11 ` Pedro Alves
@ 2013-12-02 6:41 ` Yao Qi
2013-12-02 10:02 ` Pedro Alves
0 siblings, 1 reply; 4+ messages in thread
From: Yao Qi @ 2013-12-02 6:41 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 11/30/2013 04:57 PM, Pedro Alves wrote:
> Instead of setting and unsetting NC at each iteration, the "canonical" way
> for this sort of thing is to check whether the loop ended:
>
> - if (nc == NULL)
> + for (i == ARRAY_SIZE (notifs))
>
> And then again, we don't really need NC in the loop. I think
> this would look cleaner and clearer:
OK, you've fixed that bug :). Pushed it for you.
--
Yao (é½å°§)
Subject: [PATCH] Fix PR remote/15974
In remote-notif.c:handle_notification, we have a loop,
for (i = 0; i < ARRAY_SIZE (notifs); i++)
{
nc = notifs[i];
if (strncmp (buf, nc->name, strlen (nc->name)) == 0
&& buf[strlen (nc->name)] == ':')
break;
}
/* We ignore notifications we don't recognize, for compatibility
with newer stubs. */
if (nc == NULL)
return;
If the notification is not in the list 'notifs', the last entry is
used, which is wrong. It should be NULL. This patch fixes it.
gdb:
2013-12-02 Pedro Alves <palves@redhat.com>
PR remote/15974
* remote-notif.c (handle_notification): Return early if no
notification is found.
---
gdb/remote-notif.c | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
index 0d59279..163979d 100644
--- a/gdb/remote-notif.c
+++ b/gdb/remote-notif.c
@@ -127,22 +127,25 @@ remote_async_get_pending_events_handler (gdb_client_data data)
void
handle_notification (struct remote_notif_state *state, char *buf)
{
- struct notif_client *nc = NULL;
- int i;
+ struct notif_client *nc;
+ size_t i;
for (i = 0; i < ARRAY_SIZE (notifs); i++)
{
- nc = notifs[i];
- if (strncmp (buf, nc->name, strlen (nc->name)) == 0
- && buf[strlen (nc->name)] == ':')
+ const char *name = notifs[i]->name;
+
+ if (strncmp (buf, name, strlen (name)) == 0
+ && buf[strlen (name)] == ':')
break;
}
/* We ignore notifications we don't recognize, for compatibility
with newer stubs. */
- if (nc == NULL)
+ if (i == ARRAY_SIZE (notifs))
return;
+ nc = notifs[i];
+
if (state->pending_event[nc->id] != NULL)
{
/* We've already parsed the in-flight reply, but the stub for some
--
1.7.7.6
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] Fix PR remote/15974
2013-12-02 6:41 ` Yao Qi
@ 2013-12-02 10:02 ` Pedro Alves
0 siblings, 0 replies; 4+ messages in thread
From: Pedro Alves @ 2013-12-02 10:02 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 12/02/2013 06:39 AM, Yao Qi wrote:
> On 11/30/2013 04:57 PM, Pedro Alves wrote:
>> Instead of setting and unsetting NC at each iteration, the "canonical" way
>> for this sort of thing is to check whether the loop ended:
>>
>> - if (nc == NULL)
>> + for (i == ARRAY_SIZE (notifs))
>>
>> And then again, we don't really need NC in the loop. I think
>> this would look cleaner and clearer:
>
> OK, you've fixed that bug :). Pushed it for you.
Thanks.
--
Pedro Alves
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-12-02 10:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-30 5:47 [PATCH] Fix PR remote/15974 Yao Qi
2013-11-30 10:11 ` Pedro Alves
2013-12-02 6:41 ` Yao Qi
2013-12-02 10:02 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox