From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13343 invoked by alias); 30 Nov 2013 08:58:08 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 13334 invoked by uid 89); 30 Nov 2013 08:58:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.3 required=5.0 tests=AWL,BAYES_50,RDNS_NONE,SPF_HELO_PASS,SPF_PASS,URIBL_BLOCKED autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from Unknown (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 30 Nov 2013 08:58:06 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rAU8vw30004754 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sat, 30 Nov 2013 03:57:58 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id rAU8vutw002276; Sat, 30 Nov 2013 03:57:57 -0500 Message-ID: <5299A894.70100@redhat.com> Date: Sat, 30 Nov 2013 10:11:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Yao Qi CC: gdb-patches@sourceware.org Subject: Re: [PATCH] Fix PR remote/15974 References: <1385782688-375-1-git-send-email-yao@codesourcery.com> In-Reply-To: <1385782688-375-1-git-send-email-yao@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-11/txt/msg00957.txt.bz2 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 > > 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