- updated NAT callback function defintion to no longer carry around strings,

- NAT lib now uses error-code enums to signal success and errors
- reworked related code to return errorcodes
- added and updated a missing doxygen documention
- todo next: testcases

6 files changed, 94 insertions, 50 deletions

diff --git a/src/include/gnunet_nat_lib.h b/src/include/gnunet_nat_lib.h
index d012353b5..40a5135bd 100644
--- a/src/include/gnunet_nat_lib.h
+++ b/src/include/gnunet_nat_lib.h
@@ -71,6 +71,56 @@ struct GNUNET_NAT_Handle;
 
 
 /**
+ * Error Types for the NAT subsystem (which can then later be converted/resolved to a string)
+ */
+enum GNUNET_NAT_FailureCode {
+  /**
+   * Just the default
+   */
+  GNUNET_NAT_ERROR_SUCCESS = 0,
+  
+  /**
+   * `upnpc` command not found
+   */
+  GNUNET_NAT_ERROR_UPNPC_NOT_FOUND,
+  
+  /**
+   * Failed to run `upnpc` command
+   */
+  GNUNET_NAT_ERROR_UPNPC_FAILED,
+  
+  /**
+   * `upnpc' command took too long, process killed
+   */
+  GNUNET_NAT_ERROR_UPNPC_TIMEOUT,
+  
+  /**
+   * `upnpc' command failed to establish port mapping
+   */
+  GNUNET_NAT_ERROR_UPNPC_PORTMAP_FAILED,
+  
+  /**
+   * `external-ip' command not found
+   */
+  GNUNET_NAT_ERROR_EXTERNAL_IP_UTILITY_NOT_FOUND,
+  
+  /**
+   * "no valid address was returned by `external-ip'"
+   */
+  GNUNET_NAT_ERROR_EXTERNAL_IP_NO_VALID_ADDRESS_FOUND,
+  
+  
+  
+  /**
+   * 
+   */
+  GNUNET_NAT_ERROR_,
+  
+  
+};
+
+
+/**
  * Attempt to enable port redirection and detect public IP address
  * contacting UPnP or NAT-PMP routers on the local network. Use addr
  * to specify to which of the local host's addresses should the
@@ -157,14 +207,10 @@ struct GNUNET_NAT_Test;
  * NAT configuration test.
  *
  * @param cls closure
- * @param success #GNUNET_OK on success, #GNUNET_NO on failure,
- *                #GNUNET_SYSERR if the test could not be
- *                properly started (internal failure)
- * @param emsg NULL on success, otherwise may include an error message
+ * @param result GNUNET_NAT_ERROR_SUCCESS on success, otherwise the specific error code
  */
 typedef void (*GNUNET_NAT_TestCallback) (void *cls,
-                                         int success,
-                                         const char *emsg);
+                                         enum GNUNET_NAT_FailureCode result);
 
 
 /**
@@ -202,11 +248,11 @@ GNUNET_NAT_test_stop (struct GNUNET_NAT_Test *tst);
  *
  * @param cls closure
  * @param addr the address, NULL on errors
- * @param emsg NULL on success, otherwise may include an error message
+ * @param result GNUNET_NAT_ERROR_SUCCESS on success, otherwise the specific error code
  */
 typedef void (*GNUNET_NAT_IPCallback) (void *cls,
                                        const struct in_addr *addr,
-                                       const char *emsg);
+                                       enum GNUNET_NAT_FailureCode result);
 
 
 
@@ -254,13 +300,14 @@ struct GNUNET_NAT_MiniHandle;
  *     the previous (now invalid) one
  * @param addr either the previous or the new public IP address
  * @param addrlen actual length of the @a addr
+ * @param result GNUNET_NAT_ERROR_SUCCESS on success, otherwise the specific error code
  */
 typedef void
 (*GNUNET_NAT_MiniAddressCallback) (void *cls,
                                    int add_remove,
                                    const struct sockaddr *addr,
                                    socklen_t addrlen,
-                                   const char *emsg);
+                                   enum GNUNET_NAT_FailureCode result);
 
 
 /**
@@ -307,12 +354,12 @@ struct GNUNET_NAT_AutoHandle;
  * @param cls closure
  * @param diff minimal suggested changes to the original configuration
  *             to make it work (as best as we can)
- * @param emsg NULL on success, otherwise may include an error message
+ * @param result GNUNET_NAT_ERROR_SUCCESS on success, otherwise the specific error code
  */
 typedef void
 (*GNUNET_NAT_AutoResultCallback)(void *cls,
                                  const struct GNUNET_CONFIGURATION_Handle *diff,
-                                 const char *emsg);
+                                 enum GNUNET_NAT_FailureCode result);
 
 
 /**
diff --git a/src/nat/nat.c b/src/nat/nat.c
index 039544d57..670ae6a27 100644
--- a/src/nat/nat.c
+++ b/src/nat/nat.c
@@ -1011,20 +1011,31 @@ resolve_dns (void *cls,
  *     the previous (now invalid) one
  * @param addr either the previous or the new public IP address
  * @param addrlen actual lenght of @a addr
- * @param emsg NULL on success, otherwise an error message
+ * @param ret GNUNET_NAT_ERROR_SUCCESS on success, otherwise an error code
  */
 static void
 upnp_add (void *cls,
           int add_remove,
           const struct sockaddr *addr,
           socklen_t addrlen,
-          const char *emsg)
+          enum GNUNET_NAT_FailureCode ret)
 {
   struct GNUNET_NAT_Handle *h = cls;
   struct LocalAddressList *pos;
   struct LocalAddressList *next;
 
 
+  if (GNUNET_NAT_ERROR_SUCCESS != ret)
+  {
+    /* Error while running upnp client */
+    LOG (GNUNET_ERROR_TYPE_ERROR,
+          _("Error while running upnp client:\n"));
+
+    //FIXME: convert error code to string
+    
+    return;
+  }
+  
   if (GNUNET_YES == add_remove)
   {
     add_to_address_list (h, LAL_UPNP, addr, addrlen);
@@ -1053,17 +1064,6 @@ upnp_add (void *cls,
          GNUNET_a2s(addr, addrlen));
     GNUNET_break (0);
   }
-  else if (GNUNET_SYSERR == add_remove)
-  {
-    /* Error while running upnp client */
-    if (NULL != emsg)
-      LOG (GNUNET_ERROR_TYPE_ERROR,
-          _("Error while running upnp client: `%s'\n"), emsg);
-    else
-      LOG (GNUNET_ERROR_TYPE_ERROR,
-          _("Error while running upnp client \n"));
-    return;
-  }
   else
   {
 
diff --git a/src/nat/nat_auto.c b/src/nat/nat_auto.c
index 070099b23..184f23538 100644
--- a/src/nat/nat_auto.c
+++ b/src/nat/nat_auto.c
@@ -189,8 +189,7 @@ fail_timeout (void *cls,
  */
 static void
 result_callback (void *cls,
-                 int success,
-                 const char *emsg)
+                 enum GNUNET_NAT_FailureCode ret)
 {
   struct GNUNET_NAT_AutoHandle *ah = cls;
 
@@ -199,11 +198,11 @@ result_callback (void *cls,
   GNUNET_NAT_test_stop (ah->tst);
   ah->tst = NULL;
   GNUNET_log (GNUNET_ERROR_TYPE_INFO,
-              success
+              ret
               ? _("NAT traversal with ICMP Server succeeded.\n")
               : _("NAT traversal with ICMP Server failed.\n"));
   GNUNET_CONFIGURATION_set_value_string (ah->cfg, "nat", "ENABLE_ICMP_SERVER",
-                                         success ? "YES": "NO");
+                                         ret ? "NO" : "YES");
   next_phase (ah);
 }
 
@@ -258,7 +257,7 @@ test_online (struct GNUNET_NAT_AutoHandle *ah)
 static void
 set_external_ipv4 (void *cls,
                    const struct in_addr *addr,
-                   const char *emsg)
+                   enum GNUNET_NAT_FailureCode ret)
 {
   struct GNUNET_NAT_AutoHandle *ah = cls;
   char buf[INET_ADDRSTRLEN];
@@ -525,7 +524,7 @@ next_phase (struct GNUNET_NAT_AutoHandle *ah)
                                           ah->cfg);
     ah->fin_cb (ah->fin_cb_cls,
                 diff,
-                NULL);
+                GNUNET_NAT_ERROR_SUCCESS);
     GNUNET_CONFIGURATION_destroy (diff);
     GNUNET_NAT_autoconfig_cancel (ah);
     return;
diff --git a/src/nat/nat_mini.c b/src/nat/nat_mini.c
index 877c385d5..27a1361b2 100644
--- a/src/nat/nat_mini.c
+++ b/src/nat/nat_mini.c
@@ -150,9 +150,7 @@ read_external_ipv4 (void *cls,
           (GNUNET_OK == iret)
           ? &addr :
           NULL,
-          (GNUNET_OK == iret)
-          ? NULL
-          : _("no valid address was returned by `external-ip'"));
+          GNUNET_NAT_ERROR_EXTERNAL_IP_NO_VALID_ADDRESS_FOUND);
   GNUNET_NAT_mini_get_external_ipv4_cancel (eh);
 }
 
@@ -172,7 +170,7 @@ signal_external_ip_error (void *cls,
   eh->task = GNUNET_SCHEDULER_NO_TASK;
   eh->cb (eh->cb_cls,
           NULL,
-          _("`external-ip' command not found"));
+          GNUNET_NAT_ERROR_EXTERNAL_IP_UTILITY_NOT_FOUND);
   GNUNET_free (eh);
 }
 
@@ -363,7 +361,7 @@ run_upnpc_r (struct GNUNET_NAT_MiniHandle *mini)
     mini->ac (mini->ac_cls,
               GNUNET_SYSERR,
               NULL, 0,
-              _("Failed to run `upnpc` command"));
+              GNUNET_NAT_ERROR_UPNPC_FAILED);
     return;
   }
 }
@@ -398,7 +396,7 @@ process_refresh_output (void *cls, const char *line)
                   GNUNET_NO,
                   (const struct sockaddr *) &mini->current_addr,
                   sizeof (mini->current_addr),
-                  NULL);
+                  GNUNET_NAT_ERROR_SUCCESS);
         mini->did_map = GNUNET_NO;
       }
       run_upnpc_r (mini);
@@ -423,12 +421,12 @@ process_refresh_output (void *cls, const char *line)
     mini->ac (mini->ac_cls, GNUNET_NO,
               (const struct sockaddr *) &mini->current_addr,
               sizeof (mini->current_addr),
-              NULL);
+              GNUNET_NAT_ERROR_SUCCESS);
     mini->current_addr.sin_addr = exip;
     mini->ac (mini->ac_cls, GNUNET_YES,
               (const struct sockaddr *) &mini->current_addr,
               sizeof (mini->current_addr),
-              NULL);
+              GNUNET_NAT_ERROR_SUCCESS);
     return;
   }
   /*
@@ -460,12 +458,12 @@ process_refresh_output (void *cls, const char *line)
   mini->ac (mini->ac_cls, GNUNET_NO,
             (const struct sockaddr *) &mini->current_addr,
             sizeof (mini->current_addr),
-            NULL);
+            GNUNET_NAT_ERROR_SUCCESS);
   mini->current_addr.sin_port = htons ((uint16_t) nport);
   mini->ac (mini->ac_cls, GNUNET_YES,
             (const struct sockaddr *) &mini->current_addr,
             sizeof (mini->current_addr),
-            NULL);
+            GNUNET_NAT_ERROR_SUCCESS);
 }
 
 
@@ -509,7 +507,7 @@ do_refresh (void *cls, const struct GNUNET_SCHEDULER_TaskContext *tc)
     mini->ac (mini->ac_cls,
               GNUNET_SYSERR,
               NULL, 0,
-              _("`upnpc' command took too long, process killed"));
+              GNUNET_NAT_ERROR_UPNPC_TIMEOUT);
 }
 
 
@@ -537,7 +535,7 @@ process_map_output (void *cls,
       mini->ac (mini->ac_cls,
                 GNUNET_SYSERR,
                 NULL, 0,
-                _("`upnpc' command failed to establish port mapping"));
+                GNUNET_NAT_ERROR_UPNPC_PORTMAP_FAILED);
     if (GNUNET_SCHEDULER_NO_TASK == mini->refresh_task)
       mini->refresh_task =
         GNUNET_SCHEDULER_add_delayed (MAP_REFRESH_FREQ, &do_refresh, mini);
@@ -572,7 +570,7 @@ process_map_output (void *cls,
   mini->ac (mini->ac_cls, GNUNET_YES,
             (const struct sockaddr *) &mini->current_addr,
             sizeof (mini->current_addr),
-            NULL);
+            GNUNET_NAT_ERROR_SUCCESS);
 }
 
 
@@ -605,7 +603,7 @@ GNUNET_NAT_mini_map_start (uint16_t port,
     ac (ac_cls,
         GNUNET_SYSERR,
         NULL, 0,
-        _("`upnpc` command not found"));
+        GNUNET_NAT_ERROR_UPNPC_NOT_FOUND);
     return NULL;
   }
   LOG (GNUNET_ERROR_TYPE_DEBUG,
@@ -682,7 +680,7 @@ GNUNET_NAT_mini_map_stop (struct GNUNET_NAT_MiniHandle *mini)
   mini->ac (mini->ac_cls, GNUNET_NO,
             (const struct sockaddr *) &mini->current_addr,
             sizeof (mini->current_addr),
-            NULL);
+            GNUNET_NAT_ERROR_SUCCESS);
   /* Note: oddly enough, deletion uses the external port whereas
    * addition uses the internal port; this rarely matters since they
    * often are the same, but it might... */
diff --git a/src/nat/nat_test.c b/src/nat/nat_test.c
index 866c15e5e..7d9d1a49c 100644
--- a/src/nat/nat_test.c
+++ b/src/nat/nat_test.c
@@ -187,7 +187,7 @@ reversal_cb (void *cls,
     return;                     /* wrong port */
   }
   /* report success */
-  h->report (h->report_cls, GNUNET_OK, NULL);
+  h->report (h->report_cls, GNUNET_NAT_ERROR_SUCCESS);
 }
 
 
@@ -215,7 +215,7 @@ do_udp_read (void *cls,
        GNUNET_NETWORK_socket_recv (tst->lsock, &data, sizeof (data))))
   {
     if (data == tst->data)
-      tst->report (tst->report_cls, GNUNET_OK, NULL);
+      tst->report (tst->report_cls, GNUNET_NAT_ERROR_SUCCESS);
     else
       LOG (GNUNET_ERROR_TYPE_DEBUG,
            "Received data mismatches expected value\n");
@@ -250,7 +250,7 @@ do_read (void *cls,
        GNUNET_NETWORK_socket_recv (na->sock, &data, sizeof (data))))
   {
     if (data == tst->data)
-      tst->report (tst->report_cls, GNUNET_OK, NULL);
+      tst->report (tst->report_cls, GNUNET_NAT_ERROR_SUCCESS);
     else
       LOG (GNUNET_ERROR_TYPE_DEBUG,
            "Received data does not match expected value\n");
diff --git a/src/transport/gnunet-transport.c b/src/transport/gnunet-transport.c
index 28c4c84fb..4487fc0e7 100644
--- a/src/transport/gnunet-transport.c
+++ b/src/transport/gnunet-transport.c
@@ -545,10 +545,10 @@ display_test_result (struct TestContext *tc, enum TestResult result)
  * @param emsg error message, NULL on success
  */
 static void
-result_callback (void *cls, int success, const char *emsg)
+result_callback (void *cls, enum GNUNET_NAT_FailureCode result)
 {
   struct TestContext *tc = cls;
-  display_test_result (tc, success);
+  display_test_result (tc, result);
 }
 
 /**