Re: [PATCH 12/24] Introduce generic command options framework

[Pedro Alves <palves@redhat.com>]

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 <palves@redhat.com>
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<const option_def> 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<typename Context>
 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<typename Context>
 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<typename Context>
 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
 
-- 
2.14.5