From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x342.google.com (mail-wm1-x342.google.com [IPv6:2a00:1450:4864:20::342]) by sourceware.org (Postfix) with ESMTPS id 3C87D3857C62 for ; Mon, 5 Oct 2020 09:08:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3C87D3857C62 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wm1-x342.google.com with SMTP id e2so7960243wme.1 for ; Mon, 05 Oct 2020 02:08:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=DdDd6+LVR+DTctpiwHKG0i+5lp/+5j582kp1IWrzEsA=; b=NL3E9xlcet5CyhP51kj+nrJi77NXi9RraZJAHNVkP+8PGBtLU+fMg5Oqyf4bETR3Xd 8NIEdx8u8D0MQL0h1xXEV5i4E/uys0tbi0a7ydUUE1KI7HYDvf6o31lAOQG4aitWa4dj 7g6FfK7wiafDzGq8NCsGbRBSOEjaJBOzMeXbhXXSw/ZWj/ZgNefzaw8yzZi5+zKUUKoJ 2tnZqbobONAkNzodL5hkaoIQIi2xW/CMxADkEmKOPCDfiAUbrF+hOtXrpQvI1ag5bV39 0sLDieJwcIk+XXDl3004vP4ByprueFdjm0RlQ/Uc48D8ee8ESqB98WzRd41VC9kALahO pKsA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=DdDd6+LVR+DTctpiwHKG0i+5lp/+5j582kp1IWrzEsA=; b=fOkCSxMUGK9l9JBBq6DIZ+Xc07sgKvXx0jO/puC9dSGCVseqF0EhuCvtND+tNEOfbe F53yfqBphIZy/lKaaxZoV4VRVjtcsa4pey66IXM3/IO2Hm0MoiCRg1fk9dJxh22/lFhU lIEqSEFP9aqGOrhScet5BvNmILQK6UixgRqAZ44i1p0tyRHdP5ZoRRSoVZWF8JWGweDG fXNny7Mc3xtK/rF/DI+r48tgjINh+/hfoV3euZd4C/F+jxgcyqO2xWz6yhxvPQlz3aYQ ckGbpMOATIYiSQhV7inRe5fTUjqXUk6I5egWU5n14sLDeRfIGh2C/cGt5wZdY88I0aMe 2B/w== X-Gm-Message-State: AOAM5328H4JWgBkGvc0yYtJMxd6FV7LCTECqShJdtFCvGW1oYAiSoaWL DoLyC5WcovQ1Xb9xHzUijdOcYlXY0zru+A== X-Google-Smtp-Source: ABdhPJzku8n8qWeKK1mk/bHu2NvZjsHIX8AOSqPdmUHYs17rO4CpAyHbqXmmvBJaomjcm0DDqI4Hxw== X-Received: by 2002:a7b:cbd1:: with SMTP id n17mr4790330wmi.120.1601888886148; Mon, 05 Oct 2020 02:08:06 -0700 (PDT) Received: from localhost (host109-151-14-50.range109-151.btcentralplus.com. [109.151.14.50]) by smtp.gmail.com with ESMTPSA id c21sm969616wme.36.2020.10.05.02.08.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 05 Oct 2020 02:08:05 -0700 (PDT) Date: Mon, 5 Oct 2020 10:08:04 +0100 From: Andrew Burgess To: Marco Barisione Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 1/2] Move the code to execute a cmd_list_element out from execute_command Message-ID: <20201005090804.GF605036@embecosm.com> References: <20200914093925.5442-1-mbarisione@undo.io> <20200914093925.5442-2-mbarisione@undo.io> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200914093925.5442-2-mbarisione@undo.io> X-Operating-System: Linux/5.8.12-100.fc31.x86_64 (x86_64) X-Uptime: 10:04:51 up 2 days, 17 min, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-10.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, 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: Mon, 05 Oct 2020 09:08:09 -0000 Hi, Thanks for working on this. I have a little feedback: * Marco Barisione [2020-09-14 09:39:24 +0000]: You should imagine this patch, once applied is being looked at in isolation, without an understanding that there's a follow on patch. As such it's usually a nice idea to write a short commit message that just mentions you're performing this restructuring in preparation for a follow on patch. > gdb/ChangeLog: > > * gdbcmd.h (execute_cmd_list_command): Add declaration. > * top.c (execute_command): Move out the code to execute a > command from a cmd_list_element. > (execute_cmd_list_command): Add from code originally in > execute_command. > * top.h (execute_cmd_list_command): Add declaration. Why is execute_cmd_list_command declared twice? This doesn't feel right. Thanks, Andrew > --- > gdb/gdbcmd.h | 3 +++ > gdb/top.c | 66 ++++++++++++++++++++++++++++++---------------------- > gdb/top.h | 2 ++ > 3 files changed, 43 insertions(+), 28 deletions(-) > > diff --git a/gdb/gdbcmd.h b/gdb/gdbcmd.h > index 4406094ea5..5be474ec1a 100644 > --- a/gdb/gdbcmd.h > +++ b/gdb/gdbcmd.h > @@ -134,6 +134,9 @@ extern struct cmd_list_element *save_cmdlist; > > extern void execute_command (const char *, int); > > +extern void execute_cmd_list_command (struct cmd_list_element *, > + const char *, int); > + > /* Execute command P and returns its output. If TERM_OUT, > the output is built using terminal output behaviour such > as cli_styling. */ > diff --git a/gdb/top.c b/gdb/top.c > index acd31afb9a..8eae15a488 100644 > --- a/gdb/top.c > +++ b/gdb/top.c > @@ -581,7 +581,6 @@ execute_command (const char *p, int from_tty) > const char *arg; > std::string default_args; > std::string default_args_and_arg; > - int was_sync = current_ui->prompt_state == PROMPT_BLOCKED; > > line = p; > > @@ -641,33 +640,7 @@ execute_command (const char *p, int from_tty) > if (c->deprecated_warn_user) > deprecated_cmd_warning (line); > > - /* c->user_commands would be NULL in the case of a python command. */ > - if (c->theclass == class_user && c->user_commands) > - execute_user_command (c, arg); > - else if (c->theclass == class_user > - && c->prefixlist && !c->allow_unknown) > - /* If this is a user defined prefix that does not allow unknown > - (in other words, C is a prefix command and not a command > - that can be followed by its args), report the list of > - subcommands. */ > - { > - printf_unfiltered > - ("\"%.*s\" must be followed by the name of a subcommand.\n", > - (int) strlen (c->prefixname) - 1, c->prefixname); > - help_list (*c->prefixlist, c->prefixname, all_commands, gdb_stdout); > - } > - else if (c->type == set_cmd) > - do_set_command (arg, from_tty, c); > - else if (c->type == show_cmd) > - do_show_command (arg, from_tty, c); > - else if (!cmd_func_p (c)) > - error (_("That is not a command, just a help topic.")); > - else if (deprecated_call_command_hook) > - deprecated_call_command_hook (c, arg, from_tty); > - else > - cmd_func (c, arg, from_tty); > - > - maybe_wait_sync_command_done (was_sync); > + execute_cmd_list_command (c, arg, from_tty); > > /* If this command has been post-hooked, run the hook last. */ > execute_cmd_post_hook (c); > @@ -690,6 +663,43 @@ execute_command (const char *p, int from_tty) > cleanup_if_error.release (); > } > > +/* Execute the C command. */ > + > +void > +execute_cmd_list_command (struct cmd_list_element *c, const char *arg, > + int from_tty) > +{ > + bool was_sync = current_ui->prompt_state == PROMPT_BLOCKED; > + > + /* c->user_commands would be NULL in the case of a python command. */ > + if (c->theclass == class_user && c->user_commands) > + execute_user_command (c, arg); > + else if (c->theclass == class_user > + && c->prefixlist && !c->allow_unknown) > + { > + /* If this is a user defined prefix that does not allow unknown > + (in other words, C is a prefix command and not a command > + that can be followed by its args), report the list of > + subcommands. */ > + printf_unfiltered > + ("\"%.*s\" must be followed by the name of a subcommand.\n", > + (int) strlen (c->prefixname) - 1, c->prefixname); > + help_list (*c->prefixlist, c->prefixname, all_commands, gdb_stdout); > + } > + else if (c->type == set_cmd) > + do_set_command (arg, from_tty, c); > + else if (c->type == show_cmd) > + do_show_command (arg, from_tty, c); > + else if (!cmd_func_p (c)) > + error (_("That is not a command, just a help topic.")); > + else if (deprecated_call_command_hook) > + deprecated_call_command_hook (c, arg, from_tty); > + else > + cmd_func (c, arg, from_tty); > + > + maybe_wait_sync_command_done (was_sync); > +} > + > /* Run execute_command for P and FROM_TTY. Sends its output to FILE, > do not display it to the screen. BATCH_FLAG will be > temporarily set to true. */ > diff --git a/gdb/top.h b/gdb/top.h > index fd99297715..ba6c596da7 100644 > --- a/gdb/top.h > +++ b/gdb/top.h > @@ -241,6 +241,8 @@ extern void quit_force (int *, int); > extern void quit_command (const char *, int); > extern void quit_cover (void); > extern void execute_command (const char *, int); > +extern void execute_cmd_list_command (struct cmd_list_element *, > + const char *, int); > > /* If the interpreter is in sync mode (we're running a user command's > list, running command hooks or similars), and we just ran a > -- > 2.20.1 >