[or-cvs] r14004: Make set-option functions return sensible error codes from a (in tor/trunk: . src/or)

nickm at seul.org nickm at seul.org
Thu Mar 13 15:11:57 UTC 2008


Author: nickm
Date: 2008-03-13 11:11:56 -0400 (Thu, 13 Mar 2008)
New Revision: 14004

Modified:
   tor/trunk/
   tor/trunk/src/or/config.c
   tor/trunk/src/or/control.c
   tor/trunk/src/or/or.h
Log:
 r18787 at catbus:  nickm | 2008-03-13 11:11:52 -0400
 Make set-option functions return sensible error codes from an enum, not mysterious negative integers



Property changes on: tor/trunk
___________________________________________________________________
 svk:merge ticket from /tor/trunk [r18787] on 8246c3cf-6607-4228-993b-4d95d33730f1

Modified: tor/trunk/src/or/config.c
===================================================================
--- tor/trunk/src/or/config.c	2008-03-13 15:10:35 UTC (rev 14003)
+++ tor/trunk/src/or/config.c	2008-03-13 15:11:56 UTC (rev 14004)
@@ -728,7 +728,7 @@
 
 /** Change the current global options to contain <b>new_val</b> instead of
  * their current value; take action based on the new value; free the old value
- * as necessary.
+ * as necessary.  Returns 0 on success, -1 on failure.
  */
 int
 set_options(or_options_t *new_val, char **msg)
@@ -1979,13 +1979,13 @@
 /** Try assigning <b>list</b> to the global options. You do this by duping
  * options, assigning list to the new one, then validating it. If it's
  * ok, then throw out the old one and stick with the new one. Else,
- * revert to old and return failure.  Return 0 on success, -1 on bad
- * keys, -2 on bad values, -3 on bad transition, and -4 on failed-to-set.
+ * revert to old and return failure.  Return SETOPT_OK on success, or
+ * a setopt_err_t on failure.
  *
  * If not success, point *<b>msg</b> to a newly allocated string describing
  * what went wrong.
  */
-int
+setopt_err_t
 options_trial_assign(config_line_t *list, int use_defaults,
                      int clear_first, char **msg)
 {
@@ -2000,21 +2000,21 @@
 
   if (options_validate(get_options(), trial_options, 1, msg) < 0) {
     config_free(&options_format, trial_options);
-    return -2;
+    return SETOPT_ERR_PARSE; /*XXX021 make this separate. */
   }
 
   if (options_transition_allowed(get_options(), trial_options, msg) < 0) {
     config_free(&options_format, trial_options);
-    return -3;
+    return SETOPT_ERR_TRANSITION;
   }
 
   if (set_options(trial_options, msg)<0) {
     config_free(&options_format, trial_options);
-    return -4;
+    return SETOPT_ERR_SETTING;
   }
 
   /* we liked it. put it in place. */
-  return 0;
+  return SETOPT_OK;
 }
 
 /** Reset config option <b>var</b> to 0, 0.0, NULL, or the equivalent.
@@ -3709,7 +3709,7 @@
  *  * -3 for transition not allowed
  *  * -4 for error while setting the new options
  */
-int
+setopt_err_t
 options_init_from_string(const char *cf,
                          int command, const char *command_arg,
                          char **msg)
@@ -3717,7 +3717,7 @@
   or_options_t *oldoptions, *newoptions;
   config_line_t *cl;
   int retval;
-  int err = -1;
+  setopt_err_t err = SETOPT_ERR_MISC;
   tor_assert(msg);
 
   oldoptions = global_options; /* get_options unfortunately asserts if
@@ -3732,13 +3732,13 @@
   /* get config lines, assign them */
   retval = config_get_lines(cf, &cl);
   if (retval < 0) {
-    err = -2;
+    err = SETOPT_ERR_PARSE;
     goto err;
   }
   retval = config_assign(&options_format, newoptions, cl, 0, 0, msg);
   config_free_lines(cl);
   if (retval < 0) {
-    err = -2;
+    err = SETOPT_ERR_PARSE;
     goto err;
   }
 
@@ -3746,27 +3746,27 @@
   retval = config_assign(&options_format, newoptions,
                          global_cmdline_options, 0, 0, msg);
   if (retval < 0) {
-    err = -2;
+    err = SETOPT_ERR_PARSE;
     goto err;
   }
 
   /* Validate newoptions */
   if (options_validate(oldoptions, newoptions, 0, msg) < 0) {
-    err = -2;
+    err = SETOPT_ERR_PARSE; /*XXX021 make this separate.*/
     goto err;
   }
 
   if (options_transition_allowed(oldoptions, newoptions, msg) < 0) {
-    err = -3;
+    err = SETOPT_ERR_TRANSITION;
     goto err;
   }
 
   if (set_options(newoptions, msg)) {
-    err = -4;
+    err = SETOPT_ERR_SETTING;
     goto err; /* frees and replaces old options */
   }
 
-  return 0;
+  return SETOPT_OK;
 
  err:
   config_free(&options_format, newoptions);

Modified: tor/trunk/src/or/control.c
===================================================================
--- tor/trunk/src/or/control.c	2008-03-13 15:10:35 UTC (rev 14003)
+++ tor/trunk/src/or/control.c	2008-03-13 15:11:56 UTC (rev 14004)
@@ -665,7 +665,7 @@
 control_setconf_helper(control_connection_t *conn, uint32_t len, char *body,
                        int use_defaults)
 {
-  int r;
+  setopt_err_t opt_err;
   config_line_t *lines=NULL;
   char *start = body;
   char *errstring = NULL;
@@ -732,26 +732,30 @@
   }
   tor_free(config);
 
-  if ((r=options_trial_assign(lines, use_defaults,
-                              clear_first, &errstring)) < 0) {
+  if ((opt_err=options_trial_assign(lines, use_defaults,
+                              clear_first, &errstring)) != SETOPT_OK) {
     const char *msg;
     log_warn(LD_CONTROL,
              "Controller gave us config lines that didn't validate: %s",
              errstring);
-    switch (r) {
-      case -1:
+    switch (opt_err) {
+      case SETOPT_ERR_MISC:
         msg = "552 Unrecognized option";
         break;
-      case -2:
+      case SETOPT_ERR_PARSE:
         msg = "513 Unacceptable option value";
         break;
-      case -3:
+      case SETOPT_ERR_TRANSITION:
         msg = "553 Transition not allowed";
         break;
-      case -4:
+      case SETOPT_ERR_SETTING:
       default:
         msg = "553 Unable to set option";
         break;
+      case SETOPT_OK:
+        msg = "551 Internal error";
+        tor_fragile_assert();
+        break;
     }
     connection_printf_to_buf(conn, "%s: %s\r\n", msg, errstring);
     config_free_lines(lines);
@@ -869,31 +873,34 @@
 handle_control_loadconf(control_connection_t *conn, uint32_t len,
                          const char *body)
 {
-  int retval;
+  setopt_err_t retval;
   char *errstring = NULL;
   const char *msg = NULL;
   (void) len;
 
   retval = options_init_from_string(body, CMD_RUN_TOR, NULL, &errstring);
 
-  if (retval < 0) {
+  if (retval != SETOPT_OK) {
     log_warn(LD_CONTROL,
              "Controller gave us config file that didn't validate: %s",
              errstring);
     switch (retval) {
-      case -2:
+      case SETOPT_ERR_PARSE:
         msg = "552 Invalid config file";
         break;
-      case -3:
+      case SETOPT_ERR_TRANSITION:
         msg = "553 Transition not allowed";
         break;
-      case -4:
+      case SETOPT_ERR_SETTING:
         msg = "553 Unable to set option";
         break;
-      case -1:
+      case SETOPT_ERR_MISC:
       default:
         msg = "550 Unable to load config";
         break;
+      case SETOPT_OK:
+        tor_fragile_assert();
+        break;
     }
     if (*errstring)
       connection_printf_to_buf(conn, "%s: %s\r\n", msg, errstring);

Modified: tor/trunk/src/or/or.h
===================================================================
--- tor/trunk/src/or/or.h	2008-03-13 15:10:35 UTC (rev 14003)
+++ tor/trunk/src/or/or.h	2008-03-13 15:11:56 UTC (rev 14004)
@@ -2636,6 +2636,15 @@
 
 /********************************* config.c ***************************/
 
+/** An error from options_trial_assign or options_init_from_string. */
+typedef enum setopt_err_t {
+  SETOPT_OK = 0,
+  SETOPT_ERR_MISC = -1,
+  SETOPT_ERR_PARSE = -2,
+  SETOPT_ERR_TRANSITION = -3,
+  SETOPT_ERR_SETTING = -4,
+} setopt_err_t;
+
 or_options_t *get_options(void);
 int set_options(or_options_t *new_val, char **msg);
 void config_free_all(void);
@@ -2652,8 +2661,8 @@
 int is_local_IP(uint32_t ip) ATTR_PURE;
 void options_init(or_options_t *options);
 int options_init_from_torrc(int argc, char **argv);
-int options_init_from_string(const char *cf,
-                             int command, const char *command_arg, char **msg);
+setopt_err_t options_init_from_string(const char *cf,
+                            int command, const char *command_arg, char **msg);
 int option_is_recognized(const char *key);
 const char *option_get_canonical_name(const char *key);
 config_line_t *option_get_assignment(or_options_t *options,



More information about the tor-commits mailing list