commit 88d22b898efa5bfce82c614e454874d8807e2104 Author: Nick Mathewson nickm@torproject.org 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)