From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 58429 invoked by alias); 27 Jan 2016 01:32:15 -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 58400 invoked by uid 89); 27 Jan 2016 01:32:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:6723, H*f:sk:1453845, H*i:sk:1453845 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 27 Jan 2016 01:32:13 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 0A72CC0B66C5; Wed, 27 Jan 2016 01:32:12 +0000 (UTC) Received: from localhost (unused-10-15-17-51.yyz.redhat.com [10.15.17.51]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u0R1WBmu000946 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 26 Jan 2016 20:32:11 -0500 From: Sergio Durigan Junior To: Simon Marchi Cc: Subject: Re: [PATCH] Introduce skip_to_char and use it References: <1453845250-30360-1-git-send-email-simon.marchi@ericsson.com> X-URL: http://blog.sergiodj.net Date: Wed, 27 Jan 2016 01:32:00 -0000 In-Reply-To: <1453845250-30360-1-git-send-email-simon.marchi@ericsson.com> (Simon Marchi's message of "Tue, 26 Jan 2016 16:54:10 -0500") Message-ID: <878u3b24f8.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2016-01/txt/msg00667.txt.bz2 On Tuesday, January 26 2016, Simon Marchi wrote: > For a forthcoming patch, I need a "skip_to_colon" function. I noticed > there are two skip_to_semicolon (one in gdb and one in gdbserver). > > I thought we could put it in common/, and generalize it for any > character. To me, Agreed. > p = skip_to_char (p, ';'); > > is at least as clear as > > p = skip_to_semicolon (p); > > so I decided to just remove skip_to_semicolon and use skip_to_char. The problem with skip_to_char IMO is that it leads the reader to believe that this function performs the same job as strchr (in fact, my first reaction when I read this introduction was to think "why not use strchr?"). The main difference, however, is that this function actually skips everything if the char was not found. I believe a name like "skip_to_char_or_nul" may be more appropriate. What do you think? FWIW, I know that skip_to_space suffers from this same problem, so if you like this idea and others approve I can send a patch renaming it to skip_to_space_or_nul. Other than that, the patch looks good to me. > gdb/ChangeLog: > > * common/common-utils.h (skip_to_char): New declaration. > * common/common-utils.c (skip_to_char): New function. > * remote.c (skip_to_semicolon): Remove. > (remote_parse_stop_reply): Use skip_to_char instead of > skip_to_semicolon. > > gdb/gdbserver/ChangeLog: > > * server.c (skip_to_semicolon): Remove. > (process_point_options): Use skip_to_char instead of > skip_to_semicolon. > --- > gdb/common/common-utils.c | 12 ++++++++++++ > gdb/common/common-utils.h | 5 +++++ > gdb/gdbserver/server.c | 15 +++------------ > gdb/remote.c | 26 ++++++++------------------ > 4 files changed, 28 insertions(+), 30 deletions(-) > > diff --git a/gdb/common/common-utils.c b/gdb/common/common-utils.c > index 33668f3..32720b2 100644 > --- a/gdb/common/common-utils.c > +++ b/gdb/common/common-utils.c > @@ -288,3 +288,15 @@ skip_to_space_const (const char *chp) > chp++; > return chp; > } > + > +char * > +skip_to_char (char *chp, char to) > +{ > + if (chp == NULL) > + return NULL; > + > + while (*chp != '\0' && *chp != to) > + chp++; > + > + return chp; > +} > diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h > index 47def11..67b9df7 100644 > --- a/gdb/common/common-utils.h > +++ b/gdb/common/common-utils.h > @@ -97,4 +97,9 @@ extern const char *skip_spaces_const (const char *inp); > > extern const char *skip_to_space_const (const char *inp); > > +/* Skip characters until CHP points to a character with value TO, or the end of > + the string. Return the updated pointer. If CHP is NULL, return NULL. */ > + > +extern char *skip_to_char (char *chp, char to); > + > #endif > diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c > index 301dea9..6d70ee7 100644 > --- a/gdb/gdbserver/server.c > +++ b/gdb/gdbserver/server.c > @@ -3820,15 +3820,6 @@ main (int argc, char *argv[]) > gdb_assert_not_reached ("captured_main should never return"); > } > > -/* Skip PACKET until the next semi-colon (or end of string). */ > - > -static void > -skip_to_semicolon (char **packet) > -{ > - while (**packet != '\0' && **packet != ';') > - (*packet)++; > -} > - > /* Process options coming from Z packets for a breakpoint. PACKET is > the packet buffer. *PACKET is updated to point to the first char > after the last processed option. */ > @@ -3856,7 +3847,7 @@ process_point_options (struct breakpoint *bp, char **packet) > if (debug_threads) > debug_printf ("Found breakpoint condition.\n"); > if (!add_breakpoint_condition (bp, &dataptr)) > - skip_to_semicolon (&dataptr); > + dataptr = skip_to_char (dataptr, ';'); > } > else if (startswith (dataptr, "cmds:")) > { > @@ -3866,14 +3857,14 @@ process_point_options (struct breakpoint *bp, char **packet) > persist = (*dataptr == '1'); > dataptr += 2; > if (add_breakpoint_commands (bp, &dataptr, persist)) > - skip_to_semicolon (&dataptr); > + dataptr = skip_to_char (dataptr, ';'); > } > else > { > fprintf (stderr, "Unknown token %c, ignoring.\n", > *dataptr); > /* Skip tokens until we find one that we recognize. */ > - skip_to_semicolon (&dataptr); > + dataptr = skip_to_char (dataptr, ';'); > } > } > *packet = dataptr; > diff --git a/gdb/remote.c b/gdb/remote.c > index b0303f6..a65a384 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -6387,16 +6387,6 @@ peek_stop_reply (ptid_t ptid) > stop_reply_match_ptid_and_ws, &ptid); > } > > -/* Skip PACKET until the next semi-colon (or end of string). */ > - > -static char * > -skip_to_semicolon (char *p) > -{ > - while (*p != '\0' && *p != ';') > - p++; > - return p; > -} > - > /* Helper for remote_parse_stop_reply. Return nonzero if the substring > starting with P and ending with PEND matches PREFIX. */ > > @@ -6499,7 +6489,7 @@ Packet: '%s'\n"), > /* The value part is documented as "must be empty", > though we ignore it, in case we ever decide to make > use of it in a backward compatible way. */ > - p = skip_to_semicolon (p1 + 1); > + p = skip_to_char (p1 + 1, ';'); > } > else if (strprefix (p, p1, "hwbreak")) > { > @@ -6511,19 +6501,19 @@ Packet: '%s'\n"), > error (_("Unexpected hwbreak stop reason")); > > /* See above. */ > - p = skip_to_semicolon (p1 + 1); > + p = skip_to_char (p1 + 1, ';'); > } > else if (strprefix (p, p1, "library")) > { > event->ws.kind = TARGET_WAITKIND_LOADED; > - p = skip_to_semicolon (p1 + 1); > + p = skip_to_char (p1 + 1, ';'); > } > else if (strprefix (p, p1, "replaylog")) > { > event->ws.kind = TARGET_WAITKIND_NO_HISTORY; > /* p1 will indicate "begin" or "end", but it makes > no difference for now, so ignore it. */ > - p = skip_to_semicolon (p1 + 1); > + p = skip_to_char (p1 + 1, ';'); > } > else if (strprefix (p, p1, "core")) > { > @@ -6545,7 +6535,7 @@ Packet: '%s'\n"), > else if (strprefix (p, p1, "vforkdone")) > { > event->ws.kind = TARGET_WAITKIND_VFORK_DONE; > - p = skip_to_semicolon (p1 + 1); > + p = skip_to_char (p1 + 1, ';'); > } > else if (strprefix (p, p1, "exec")) > { > @@ -6574,7 +6564,7 @@ Packet: '%s'\n"), > else if (strprefix (p, p1, "create")) > { > event->ws.kind = TARGET_WAITKIND_THREAD_CREATED; > - p = skip_to_semicolon (p1 + 1); > + p = skip_to_char (p1 + 1, ';'); > } > else > { > @@ -6583,7 +6573,7 @@ Packet: '%s'\n"), > > if (skipregs) > { > - p = skip_to_semicolon (p1 + 1); > + p = skip_to_char (p1 + 1, ';'); > p++; > continue; > } > @@ -6620,7 +6610,7 @@ Packet: '%s'\n"), > { > /* Not a number. Silently skip unknown optional > info. */ > - p = skip_to_semicolon (p1 + 1); > + p = skip_to_char (p1 + 1, ';'); > } > } > > -- > 2.5.1 -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/