[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [tor/master] Simplify handler logic in control_cmd.c



commit 88d22b898efa5bfce82c614e454874d8807e2104
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date:   Tue Apr 9 10:58:42 2019 -0400

    Simplify handler logic in control_cmd.c
    
    Now that the legacy handlers are gone, we can simplify the
    structures and macros here.
---
 src/feature/control/control_cmd.c | 188 +++++++++++++-------------------------
 1 file changed, 66 insertions(+), 122 deletions(-)

diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c
index a29ab72ff..c3863c646 100644
--- a/src/feature/control/control_cmd.c
+++ b/src/feature/control/control_cmd.c
@@ -2171,16 +2171,18 @@ handle_control_del_onion(control_connection_t *conn,
   return 0;
 }
 
+static const control_cmd_syntax_t obsolete_syntax = {
+  .max_args = UINT_MAX
+};
+
 /**
  * Called when we get an obsolete command: tell the controller that it is
  * obsolete.
  */
 static int
 handle_control_obsolete(control_connection_t *conn,
-                        uint32_t arg_len,
-                        const char *args)
+                        const control_cmd_args_t *args)
 {
-  (void)arg_len;
   (void)args;
   char *command = tor_strdup(conn->current_cmd);
   tor_strupper(command);
@@ -2190,37 +2192,10 @@ handle_control_obsolete(control_connection_t *conn,
 }
 
 /**
- * Selects an API to a controller command.  See handler_fn_t for the
- * possible types.
- **/
-typedef enum handler_type_t {
-  hnd_legacy,
-  hnd_parsed,
-} handler_type_t;
-
-/**
- * Union: a function pointer to a handler function for a controller command.
- *
- * This needs to be a union (rather than just a single pointer) since not
- * all controller commands have the same type.
+ * Function pointer to a handler function for a controller command.
  **/
-typedef union handler_fn_t {
-  /**
-   * A "legacy" handler takes a command's arguments as a nul-terminated
-   * string, and their length.  It may not change the contents of the
-   * arguments.  If the command is a multiline one, then the arguments may
-   * extend across multiple lines.
-   */
-  int (*legacy)(control_connection_t *conn,
-                uint32_t arg_len,
-                const char *args);
-  /**
-   * A "parsed" handler expects its arguments in a pre-parsed format, in
-   * an immutable control_cmd_args_t *object.
-   **/
-  int (*parsed)(control_connection_t *conn,
-                const control_cmd_args_t *args);
-} handler_fn_t;
+typedef int (*handler_fn_t) (control_connection_t *conn,
+                             const control_cmd_args_t *args);
 
 /**
  * Definition for a controller command.
@@ -2231,10 +2206,6 @@ typedef struct control_cmd_def_t {
    * begin with "+".  This is not case-sensitive. */
   const char *name;
   /**
-   * Which API to use when calling the handler function.
-   */
-  handler_type_t handler_type;
-  /**
    * A function to execute the command.
    */
   handler_fn_t handler;
@@ -2254,54 +2225,37 @@ typedef struct control_cmd_def_t {
  */
 #define CMD_FL_WIPE (1u<<0)
 
-#define SYNTAX_IGNORE { 0, UINT_MAX, false }
-
 /** Macro: declare a command with a one-line argument, a given set of flags,
  * and a syntax definition.
  **/
-#define ONE_LINE_(name, htype, flags, syntax)                   \
-  { #name,                                                      \
-      hnd_ ##htype,                                             \
-      { .htype = handle_control_ ##name },                      \
+#define ONE_LINE(name, flags)                                   \
+  {                                                             \
+    #name,                                                      \
+      handle_control_ ##name,                                   \
       flags,                                                    \
-      syntax,                                                   \
-  }
-
-/** Macro: declare a parsed command with a one-line argument, a given set of
- * flags, and a syntax definition.
- **/
-#define ONE_LINE(name, htype, flags) \
-  ONE_LINE_(name, htype, flags, NULL)
-#define ONE_LINE_PARSED(name, flags) \
-  ONE_LINE_(name, parsed, flags, &name ##_syntax)
+      &name##_syntax,                                           \
+   }
 
 /**
  * Macro: declare a command with a multi-line argument and a given set of
  * flags.
  **/
-#define MULTLINE_(name, htype, flags, syntax)                   \
+#define MULTLINE(name, flags)                                   \
   { "+"#name,                                                   \
-      hnd_ ##htype,                                             \
-      { .htype = handle_control_ ##name },                      \
+      handle_control_ ##name,                                   \
       flags,                                                    \
-      syntax                                                    \
+      &name##_syntax                                            \
   }
 
-#define MULTLINE(name, htype, flags) \
-  MULTLINE_(name, htype, flags, NULL)
-#define MULTLINE_PARSED(name, flags)                   \
-  MULTLINE_(name, parsed, flags, &name##_syntax)
-
 /**
  * Macro: declare an obsolete command. (Obsolete commands give a different
  * error than non-existent ones.)
  **/
 #define OBSOLETE(name)                          \
   { #name,                                      \
-      hnd_legacy,                               \
-      { .legacy = handle_control_obsolete },    \
+      handle_control_obsolete,                  \
       0,                                        \
-      NULL,                                     \
+      &obsolete_syntax,                         \
   }
 
 /**
@@ -2309,35 +2263,35 @@ typedef struct control_cmd_def_t {
  **/
 static const control_cmd_def_t CONTROL_COMMANDS[] =
 {
-  ONE_LINE_PARSED(setconf, 0),
-  ONE_LINE_PARSED(resetconf, 0),
-  ONE_LINE_PARSED(getconf, 0),
-  MULTLINE_PARSED(loadconf, 0),
-  ONE_LINE_PARSED(setevents, 0),
-  ONE_LINE_PARSED(authenticate, CMD_FL_WIPE),
-  ONE_LINE_PARSED(saveconf, 0),
-  ONE_LINE_PARSED(signal, 0),
-  ONE_LINE_PARSED(takeownership, 0),
-  ONE_LINE_PARSED(dropownership, 0),
-  ONE_LINE_PARSED(mapaddress, 0),
-  ONE_LINE_PARSED(getinfo, 0),
-  ONE_LINE_PARSED(extendcircuit, 0),
-  ONE_LINE_PARSED(setcircuitpurpose, 0),
+  ONE_LINE(setconf, 0),
+  ONE_LINE(resetconf, 0),
+  ONE_LINE(getconf, 0),
+  MULTLINE(loadconf, 0),
+  ONE_LINE(setevents, 0),
+  ONE_LINE(authenticate, CMD_FL_WIPE),
+  ONE_LINE(saveconf, 0),
+  ONE_LINE(signal, 0),
+  ONE_LINE(takeownership, 0),
+  ONE_LINE(dropownership, 0),
+  ONE_LINE(mapaddress, 0),
+  ONE_LINE(getinfo, 0),
+  ONE_LINE(extendcircuit, 0),
+  ONE_LINE(setcircuitpurpose, 0),
   OBSOLETE(setrouterpurpose),
-  ONE_LINE_PARSED(attachstream, 0),
-  MULTLINE_PARSED(postdescriptor, 0),
-  ONE_LINE_PARSED(redirectstream, 0),
-  ONE_LINE_PARSED(closestream, 0),
-  ONE_LINE_PARSED(closecircuit, 0),
-  ONE_LINE_PARSED(usefeature, 0),
-  ONE_LINE_PARSED(resolve, 0),
-  ONE_LINE_PARSED(protocolinfo, 0),
-  ONE_LINE_PARSED(authchallenge, CMD_FL_WIPE),
-  ONE_LINE_PARSED(dropguards, 0),
-  ONE_LINE_PARSED(hsfetch, 0),
-  MULTLINE_PARSED(hspost, 0),
-  ONE_LINE_PARSED(add_onion, CMD_FL_WIPE),
-  ONE_LINE_PARSED(del_onion, CMD_FL_WIPE),
+  ONE_LINE(attachstream, 0),
+  MULTLINE(postdescriptor, 0),
+  ONE_LINE(redirectstream, 0),
+  ONE_LINE(closestream, 0),
+  ONE_LINE(closecircuit, 0),
+  ONE_LINE(usefeature, 0),
+  ONE_LINE(resolve, 0),
+  ONE_LINE(protocolinfo, 0),
+  ONE_LINE(authchallenge, CMD_FL_WIPE),
+  ONE_LINE(dropguards, 0),
+  ONE_LINE(hsfetch, 0),
+  MULTLINE(hspost, 0),
+  ONE_LINE(add_onion, CMD_FL_WIPE),
+  ONE_LINE(del_onion, CMD_FL_WIPE),
 };
 
 /**
@@ -2356,35 +2310,25 @@ handle_single_control_command(const control_cmd_def_t *def,
                               char *args)
 {
   int rv = 0;
-  switch (def->handler_type) {
-    case hnd_legacy:
-      if (def->handler.legacy(conn, cmd_data_len, args))
-        rv = -1;
-      break;
-    case hnd_parsed: {
-      control_cmd_args_t *parsed_args;
-      char *err=NULL;
-      tor_assert(def->syntax);
-      parsed_args = control_cmd_parse_args(conn->current_cmd,
-                                           def->syntax,
-                                           cmd_data_len, args,
-                                           &err);
-      if (!parsed_args) {
-        connection_printf_to_buf(conn,
-                                 "512 Bad arguments to %s: %s\r\n",
-                                 conn->current_cmd, err?err:"");
-        tor_free(err);
-      } else {
-        if (BUG(err))
-          tor_free(err);
-        if (def->handler.parsed(conn, parsed_args))
-          rv = 0;
-        control_cmd_args_free(parsed_args);
-      }
-      break;
-    }
-    default:
-      tor_assert_unreached();
+
+  control_cmd_args_t *parsed_args;
+  char *err=NULL;
+  tor_assert(def->syntax);
+  parsed_args = control_cmd_parse_args(conn->current_cmd,
+                                       def->syntax,
+                                       cmd_data_len, args,
+                                       &err);
+  if (!parsed_args) {
+    connection_printf_to_buf(conn,
+                             "512 Bad arguments to %s: %s\r\n",
+                             conn->current_cmd, err?err:"");
+    tor_free(err);
+  } else {
+    if (BUG(err))
+      tor_free(err);
+    if (def->handler(conn, parsed_args))
+      rv = 0;
+    control_cmd_args_free(parsed_args);
   }
 
   if (def->flags & CMD_FL_WIPE)



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits