[PATCH] Fix PR remote/15974

[PATCH] Fix PR remote/15974

[Yao Qi]

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

Re: [PATCH] Fix PR remote/15974

[Pedro Alves]

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

Re: [PATCH] Fix PR remote/15974

[Yao Qi]

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

Re: [PATCH] Fix PR remote/15974

[Pedro Alves]

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