From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from beryx.lancelotsix.com (beryx.lancelotsix.com [164.132.98.193]) by sourceware.org (Postfix) with ESMTPS id 0C0ED3851C25 for ; Sun, 10 Jan 2021 00:06:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 0C0ED3851C25 Received: from gwenhwyvar (unknown [IPv6:2a02:390:8443:0:4800:6932:fb3c:137f]) by beryx.lancelotsix.com (Postfix) with ESMTPSA id 6CBA82E070; Sun, 10 Jan 2021 01:06:26 +0100 (CET) Date: Sun, 10 Jan 2021 00:06:21 +0000 From: Lancelot SIX To: gdb-patches@sourceware.org Subject: Re: [PATCH 1/4] gdb: add lookup_cmd_exact to simplify a common pattern Message-ID: <20210110000432.GA24151@gwenhwyvar> References: <20210108100706.96190-1-mbarisione@undo.io> <20210108100706.96190-2-mbarisione@undo.io> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20210108100706.96190-2-mbarisione@undo.io> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (beryx.lancelotsix.com [0.0.0.0]); Sun, 10 Jan 2021 01:06:26 +0100 (CET) X-Spam-Status: No, score=-13.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 10 Jan 2021 00:06:31 -0000 Hi I just have a few style-related remarks above. On Fri, Jan 08, 2021 at 10:07:03AM +0000, Marco Barisione via Gdb-patches wrote: > In code dealing with commands, there's a pattern repeated a few times of > calling lookup_cmd with some speficic arguments and then using strcmp > on the returned command to check for an exact match. > As a later patch would add a few more similar lines of code, this patch > adds a new lookup_cmd_exact function which simplify this use case. > > gdb/ChangeLog: > > * cli/cli-decode.c (lookup_cmd_exact): Add. > * cli/cli-script.c (do_define_command): Use lookup_cmd_exact. > (define_prefix_command): Ditto. > * command.h: Add lookup_cmd_exact. > --- > gdb/cli/cli-decode.c | 15 +++++++++++++++ > gdb/cli/cli-script.c | 23 ++++++----------------- > gdb/command.h | 19 +++++++++++++++++++ > 3 files changed, 40 insertions(+), 17 deletions(-) > > diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c > index 99bd4c6d2cd..87785107e3d 100644 > --- a/gdb/cli/cli-decode.c > +++ b/gdb/cli/cli-decode.c > @@ -1875,6 +1875,21 @@ lookup_cmd (const char **line, struct cmd_list_element *list, > return 0; > } > > +/* See command.h. */ > + > +struct cmd_list_element * > +lookup_cmd_exact (const char *name, > + struct cmd_list_element *list, > + bool ignore_help_classes) > +{ > + const char *tem = name; > + struct cmd_list_element *cmd = lookup_cmd (&tem, list, "", NULL, -1, Probably s/NULL/nullptr/ ? > + ignore_help_classes); > + if (cmd && strcmp (name, cmd->name) != 0) I think gdb prefers explicit comparison to check for null pointers: https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Comparison_With_NULL_And_Zero + if (cmd != nullptr && strcmp (name, cmd->name) != 0) BR Lancelot > + cmd = nullptr; > + return cmd; > +} > + > /* We are here presumably because an alias or command in TEXT is > deprecated and a warning message should be generated. This > function decodes TEXT and potentially generates a warning message > diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c > index 9d0dd7796e0..0544f3efb1b 100644 > --- a/gdb/cli/cli-script.c > +++ b/gdb/cli/cli-script.c > @@ -1391,7 +1391,7 @@ do_define_command (const char *comname, int from_tty, > CMD_POST_HOOK > }; > struct cmd_list_element *c, *newc, *hookc = 0, **list; > - const char *tem, *comfull; > + const char *comfull; > int hook_type = CMD_NO_HOOK; > int hook_name_size = 0; > > @@ -1403,11 +1403,7 @@ do_define_command (const char *comname, int from_tty, > comfull = comname; > list = validate_comname (&comname); > > - /* Look it up, and verify that we got an exact match. */ > - tem = comname; > - c = lookup_cmd (&tem, *list, "", NULL, -1, 1); > - if (c && strcmp (comname, c->name) != 0) > - c = 0; > + c = lookup_cmd_exact (comname, *list); > > if (c && commands == nullptr) > { > @@ -1448,11 +1444,9 @@ do_define_command (const char *comname, int from_tty, > > if (hook_type != CMD_NO_HOOK) > { > - /* Look up cmd it hooks, and verify that we got an exact match. */ > - tem = comname + hook_name_size; > - hookc = lookup_cmd (&tem, *list, "", NULL, -1, 0); > - if (hookc && strcmp (comname + hook_name_size, hookc->name) != 0) > - hookc = 0; > + /* Look up cmd it hooks. */ > + hookc = lookup_cmd_exact (comname + hook_name_size, *list, > + /* ignore_help_classes = */ false); > if (!hookc && commands == nullptr) > { > warning (_("Your new `%s' command does not " > @@ -1593,17 +1587,12 @@ static void > define_prefix_command (const char *comname, int from_tty) > { > struct cmd_list_element *c, **list; > - const char *tem; > const char *comfull; > > comfull = comname; > list = validate_comname (&comname); > > - /* Look it up, and verify that we got an exact match. */ > - tem = comname; > - c = lookup_cmd (&tem, *list, "", NULL, -1, 1); > - if (c != nullptr && strcmp (comname, c->name) != 0) > - c = nullptr; > + c = lookup_cmd_exact (comname, *list); > > if (c != nullptr && c->theclass != class_user) > error (_("Command \"%s\" is built-in."), comfull); > diff --git a/gdb/command.h b/gdb/command.h > index 79e5017ff7a..827a19637a2 100644 > --- a/gdb/command.h > +++ b/gdb/command.h > @@ -326,6 +326,25 @@ extern struct cmd_list_element *lookup_cmd_1 > struct cmd_list_element **result_list, std::string *default_args, > int ignore_help_classes, bool lookup_for_completion_p = false); > > +/* Look up the command called NAME in the command list LIST. > + > + Unlike LOOKUP_CMD, partial matches are ignored and only exact matches > + on NAME are considered. > + > + LIST is a chain of struct cmd_list_element's. > + > + If IGNORE_HELP_CLASSES is true (the default), ignore any command list > + elements which are actually help classes rather than commands (i.e. > + the function field of the struct cmd_list_element is null). > + > + If found, return the struct cmd_list_element for that command, > + otherwise return NULLPTR. */ > + > +extern struct cmd_list_element *lookup_cmd_exact > + (const char *name, > + struct cmd_list_element *list, > + bool ignore_help_classes = true); > + > extern struct cmd_list_element *deprecate_cmd (struct cmd_list_element *, > const char * ); > > -- > 2.28.0 >