From patchwork Sat May 25 10:31:29 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 32855 Received: (qmail 126690 invoked by alias); 25 May 2019 10:31:35 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 126682 invoked by uid 89); 25 May 2019 10:31:34 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-20.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=var_types X-HELO: mail-wm1-f68.google.com Received: from mail-wm1-f68.google.com (HELO mail-wm1-f68.google.com) (209.85.128.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 25 May 2019 10:31:33 +0000 Received: by mail-wm1-f68.google.com with SMTP id f10so4253950wmb.1 for ; Sat, 25 May 2019 03:31:32 -0700 (PDT) Return-Path: Received: from [192.168.1.34] ([62.28.162.51]) by smtp.gmail.com with ESMTPSA id o6sm12206079wrh.55.2019.05.25.03.31.29 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Sat, 25 May 2019 03:31:30 -0700 (PDT) Subject: Re: [PATCH 12/24] Introduce generic command options framework To: Philippe Waroquiers , gdb-patches@sourceware.org References: <20190522205327.2568-1-palves@redhat.com> <20190522205327.2568-13-palves@redhat.com> <1558770207.1454.19.camel@skynet.be> From: Pedro Alves Message-ID: <4f861909-e59d-01ae-fab7-cd8527f1fa63@redhat.com> Date: Sat, 25 May 2019 11:31:29 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <1558770207.1454.19.camel@skynet.be> On 5/25/19 8:43 AM, Philippe Waroquiers wrote: > While quickly scanning the patch, I found a few typos ... Thanks! > > On Wed, 2019-05-22 at 21:53 +0100, Pedro Alves wrote: >> diff --git a/gdb/cli/cli-option.c b/gdb/cli/cli-option.c >> new file mode 100644 >> index 00000000000..432555a953f >> --- /dev/null >> +++ b/gdb/cli/cli-option.c > ... >> +/* Helper for build_help. Appends an indended version of DOC into >> + HELP. */ > indended -> indented >> + >> +static void >> +append_indended_doc (const char *doc, std::string &help) > Same in the function name > Fixed. > >> diff --git a/gdb/cli/cli-option.h b/gdb/cli/cli-option.h >> new file mode 100644 >> index 00000000000..437fc09a61b >> --- /dev/null >> +++ b/gdb/cli/cli-option.h > ... >> +struct option_def >> +{ >> + /* The ctor is protected because you're supposed to construct using >> + one of bool_option_def, etc. below. */ >> +protected: >> + typedef void *(erased_var_address_ftype) (); >> + >> + /* Construct an option. NAME_ is the option's name. VAR_TYPE_ >> + defines the option's type. ERASED_VAR_ADDRESS_ is a pointer to >> + the option's control variable. SHOW_CMD_CB_ is a pointer to > Isn't ERASED_VAR_ADDRESS_ the address of a function ? You're right. I think it wasn't at some point... I've renamed it and everything related to include "get_" in the name. > > >> diff --git a/gdb/testsuite/gdb.base/options.exp b/gdb/testsuite/gdb.base/options.exp >> new file mode 100644 >> index 00000000000..924d7aa544e >> --- /dev/null >> +++ b/gdb/testsuite/gdb.base/options.exp > ... >> >> +# Miscelaneous tests. > Miscellaneous > Fixed. Here's the patch that I'm squashing in. From 4bce43c41d08f88c5ae598d54adec9097dbdeab3 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Sat, 25 May 2019 11:24:51 +0100 Subject: [PATCH] philippe review --- gdb/cli/cli-option.c | 8 ++++---- gdb/cli/cli-option.h | 36 ++++++++++++++++++------------------ gdb/testsuite/gdb.base/options.exp | 2 +- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/gdb/cli/cli-option.c b/gdb/cli/cli-option.c index 432555a953f..9793dd5727e 100644 --- a/gdb/cli/cli-option.c +++ b/gdb/cli/cli-option.c @@ -601,11 +601,11 @@ get_val_type_str (const option_def &opt, std::string &buffer) } } -/* Helper for build_help. Appends an indended version of DOC into +/* Helper for build_help. Appends an indented version of DOC into HELP. */ static void -append_indended_doc (const char *doc, std::string &help) +append_indented_doc (const char *doc, std::string &help) { const char *p = doc; const char *n = strchr (p, '\n'); @@ -646,9 +646,9 @@ build_help_option (gdb::array_view options, help += val_type_str; } help += "\n"; - append_indended_doc (o.set_doc, help); + append_indented_doc (o.set_doc, help); if (o.help_doc != nullptr) - append_indended_doc (o.help_doc, help); + append_indented_doc (o.help_doc, help); help += '\n'; } } diff --git a/gdb/cli/cli-option.h b/gdb/cli/cli-option.h index 437fc09a61b..1bfbfce1ce5 100644 --- a/gdb/cli/cli-option.h +++ b/gdb/cli/cli-option.h @@ -39,23 +39,23 @@ struct option_def /* The ctor is protected because you're supposed to construct using one of bool_option_def, etc. below. */ protected: - typedef void *(erased_var_address_ftype) (); + typedef void *(erased_get_var_address_ftype) (); /* Construct an option. NAME_ is the option's name. VAR_TYPE_ - defines the option's type. ERASED_VAR_ADDRESS_ is a pointer to - the option's control variable. SHOW_CMD_CB_ is a pointer to - callback for the "show" command that is installed for this - option. SET_DOC_, SHOW_DOC_, HELP_DOC_ are used to create the - option's "set/show" commands. */ + defines the option's type. ERASED_GET_VAR_ADDRESS_ is a pointer + to a function that returns the option's control variable. + SHOW_CMD_CB_ is a pointer to callback for the "show" command that + is installed for this option. SET_DOC_, SHOW_DOC_, HELP_DOC_ are + used to create the option's "set/show" commands. */ constexpr option_def (const char *name_, var_types var_type_, - erased_var_address_ftype *erased_var_address_, + erased_get_var_address_ftype *erased_get_var_address_, show_value_ftype *show_cmd_cb_, const char *set_doc_, const char *show_doc_, const char *help_doc_) : name (name_), type (var_type_), - erased_var_address (erased_var_address_), + erased_get_var_address (erased_get_var_address_), var_address {}, show_cmd_cb (show_cmd_cb_), set_doc (set_doc_), show_doc (show_doc_), help_doc (help_doc_) @@ -70,7 +70,7 @@ public: /* A function that gets the controlling variable's address, type erased. */ - erased_var_address_ftype *erased_var_address; + erased_get_var_address_ftype *erased_get_var_address; /* Get the controlling variable's address. Each type of variable uses a different union member. We do this instead of having a @@ -128,7 +128,7 @@ static inline RetType * get_var_address (const option_def &option, void *ctx) { using unerased_ftype = RetType *(Context *); - unerased_ftype *fun = (unerased_ftype *) option.erased_var_address; + unerased_ftype *fun = (unerased_ftype *) option.erased_get_var_address; return fun ((Context *) ctx); } @@ -154,13 +154,13 @@ template struct boolean_option_def : option_def { boolean_option_def (const char *long_option_, - int *(*var_address_cb_) (Context *), + int *(*get_var_address_cb_) (Context *), show_value_ftype *show_cmd_cb_, const char *set_doc_, const char *show_doc_ = nullptr, const char *help_doc_ = nullptr) : option_def (long_option_, var_boolean, - (erased_var_address_ftype *) var_address_cb_, + (erased_get_var_address_ftype *) get_var_address_cb_, show_cmd_cb_, set_doc_, show_doc_, help_doc_) { @@ -205,13 +205,13 @@ template struct uinteger_option_def : option_def { uinteger_option_def (const char *long_option_, - unsigned int *(*var_address_cb_) (Context *), + unsigned int *(*get_var_address_cb_) (Context *), show_value_ftype *show_cmd_cb_, const char *set_doc_, const char *show_doc_ = nullptr, const char *help_doc_ = nullptr) : option_def (long_option_, var_uinteger, - (erased_var_address_ftype *) var_address_cb_, + (erased_get_var_address_ftype *) get_var_address_cb_, show_cmd_cb_, set_doc_, show_doc_, help_doc_) { @@ -225,13 +225,13 @@ template struct zuinteger_unlimited_option_def : option_def { zuinteger_unlimited_option_def (const char *long_option_, - int *(*var_address_cb_) (Context *), + int *(*get_var_address_cb_) (Context *), show_value_ftype *show_cmd_cb_, const char *set_doc_, const char *show_doc_ = nullptr, const char *help_doc_ = nullptr) : option_def (long_option_, var_zuinteger_unlimited, - (erased_var_address_ftype *) var_address_cb_, + (erased_get_var_address_ftype *) get_var_address_cb_, show_cmd_cb_, set_doc_, show_doc_, help_doc_) { @@ -246,13 +246,13 @@ struct enum_option_def : option_def { enum_option_def (const char *long_option_, const char *const *enumlist, - const char **(*var_address_cb_) (Context *), + const char **(*get_var_address_cb_) (Context *), show_value_ftype *show_cmd_cb_, const char *set_doc_, const char *show_doc_ = nullptr, const char *help_doc_ = nullptr) : option_def (long_option_, var_enum, - (erased_var_address_ftype *) var_address_cb_, + (erased_get_var_address_ftype *) get_var_address_cb_, show_cmd_cb_, set_doc_, show_doc_, help_doc_) { diff --git a/gdb/testsuite/gdb.base/options.exp b/gdb/testsuite/gdb.base/options.exp index 924d7aa544e..1891176dc1d 100644 --- a/gdb/testsuite/gdb.base/options.exp +++ b/gdb/testsuite/gdb.base/options.exp @@ -117,7 +117,7 @@ set all_options { "-zuinteger-unlimited" } -# Miscelaneous tests. +# Miscellaneous tests. proc_with_prefix test-misc {variant} { global all_options