- moved timeout handling responsibility from for nat tests from caller to the library

- updated NAT API accordingly
- removed obsolete timeout handling code from gnunet-transport
- updated result callback for nat-test gnunet-transport to not always just report success, but we now actually report the proper result code we got from the NAT API
- updated nat tests

3 files changed, 40 insertions, 67 deletions

diff --git a/src/nat/nat_auto.c b/src/nat/nat_auto.c
index 3ee520d9b..5b40857af 100644
--- a/src/nat/nat_auto.c
+++ b/src/nat/nat_auto.c
@@ -158,35 +158,8 @@ next_phase (struct GNUNET_NAT_AutoHandle *ah);
 
 
 /**
- * Function called if NAT failed to confirm success.
- * Clean up and update GUI (with failure).
- *
- * @param cls closure with setup context
- * @param tc scheduler callback
- */
-static void
-fail_timeout (void *cls,
-              const struct GNUNET_SCHEDULER_TaskContext *tc)
-{
-  struct GNUNET_NAT_AutoHandle *ah = cls;
-
-  ah->ret = GNUNET_NAT_ERROR_NAT_TEST_TIMEOUT; 
-  GNUNET_log (GNUNET_ERROR_TYPE_INFO,
-              _("NAT traversal with ICMP Server timed out.\n"));
-  GNUNET_assert (NULL != ah->tst);
-  ah->task = GNUNET_SCHEDULER_NO_TASK;
-  GNUNET_NAT_test_stop (ah->tst);
-  ah->tst = NULL;
-  GNUNET_CONFIGURATION_set_value_string (ah->cfg, "nat",
-                                         "ENABLE_ICMP_SERVER",
-                                         "NO");
-  next_phase (ah);
-}
-
-
-/**
- * Function called by NAT on success.
- * Clean up and update GUI (with success).
+ * Function called by NAT to report the outcome of the nat-test.
+ * Clean up and update GUI.
  *
  * @param cls the auto handle
  * @param success currently always #GNUNET_OK
@@ -197,17 +170,15 @@ result_callback (void *cls,
                  enum GNUNET_NAT_FailureCode ret)
 {
   struct GNUNET_NAT_AutoHandle *ah = cls;
-
-  GNUNET_SCHEDULER_cancel (ah->task);
-  ah->task = GNUNET_SCHEDULER_NO_TASK;
   GNUNET_NAT_test_stop (ah->tst);
   ah->tst = NULL;
+  ah->ret = ret;
   GNUNET_log (GNUNET_ERROR_TYPE_INFO,
-              ret
+              GNUNET_NAT_ERROR_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",
-                                         ret ? "NO" : "YES");
+                                         GNUNET_NAT_ERROR_SUCCESS == ret ? "NO" : "YES");
   next_phase (ah);
 }
 
@@ -228,15 +199,8 @@ reversal_test (void *cls,
   GNUNET_log (GNUNET_ERROR_TYPE_INFO,
               _("Testing connection reversal with ICMP server.\n"));
   GNUNET_RESOLVER_connect (ah->cfg);
-  ah->tst = GNUNET_NAT_test_start (ah->cfg, GNUNET_YES, 0, 0,
+  ah->tst = GNUNET_NAT_test_start (ah->cfg, GNUNET_YES, 0, 0, TIMEOUT,
                                    &result_callback, ah);
-  if (NULL == ah->tst)
-  {
-    ah->ret = GNUNET_NAT_ERROR_NAT_TEST_START_FAILED;
-    next_phase (ah);
-    return;
-  }
-  ah->task = GNUNET_SCHEDULER_add_delayed (TIMEOUT, &fail_timeout, ah);
 }
 
 
@@ -506,15 +470,8 @@ err:
 static void
 test_icmp_client (struct GNUNET_NAT_AutoHandle *ah)
 {
-  int ext_ip;
-  int nated;
-  int binary;
   char *tmp;
   char *helper;
-  
-  ext_ip = GNUNET_NO;
-  nated = GNUNET_NO;
-  binary = GNUNET_NO;
 
   tmp = NULL;
   helper = GNUNET_OS_get_libexec_binary_path ("gnunet-helper-nat-client");
@@ -522,7 +479,6 @@ test_icmp_client (struct GNUNET_NAT_AutoHandle *ah)
         GNUNET_CONFIGURATION_get_value_string (ah->cfg, "nat", "INTERNAL_ADDRESS",
                                                &tmp)) && (0 < strlen (tmp)))
   {
-    ext_ip = GNUNET_OK;
     GNUNET_log (GNUNET_ERROR_TYPE_INFO, _("test_icmp_client not possible, as we have no internal IPv4 address\n"));
   }
   else
@@ -530,7 +486,6 @@ test_icmp_client (struct GNUNET_NAT_AutoHandle *ah)
   
   if (GNUNET_YES !=
       GNUNET_CONFIGURATION_get_value_yesno (ah->cfg, "nat", "BEHIND_NAT")){
-        nated = GNUNET_YES;
     GNUNET_log (GNUNET_ERROR_TYPE_INFO, _("test_icmp_server not possible, as we are not behind NAT\n"));
   }
   else
@@ -539,7 +494,6 @@ test_icmp_client (struct GNUNET_NAT_AutoHandle *ah)
   if (GNUNET_YES ==
       GNUNET_OS_check_helper_binary (helper, GNUNET_YES, "-d 127.0.0.1 127.0.0.2 42")){
           // none of these parameters are actually used in privilege testing mode
-    binary = GNUNET_OK;
     GNUNET_log (GNUNET_ERROR_TYPE_INFO, _("No working gnunet-helper-nat-server found\n"));
   }
 err:
diff --git a/src/nat/nat_test.c b/src/nat/nat_test.c
index 84cf3b01b..2836730ca 100644
--- a/src/nat/nat_test.c
+++ b/src/nat/nat_test.c
@@ -142,6 +142,11 @@ struct GNUNET_NAT_Test
    * Identity of task for the listen socket (if any)
    */
   GNUNET_SCHEDULER_TaskIdentifier ltask;
+  
+  /**
+   * Task identifier for the timeout (if any)
+   */
+  GNUNET_SCHEDULER_TaskIdentifier ttask;
 
   /**
    * GNUNET_YES if we're testing TCP
@@ -363,13 +368,36 @@ addr_cb (void *cls,
 
 
 /**
+ * Timeout task for a nat test. 
+ * Calls the report-callback with a timeout return value
+ * 
+ * @param cls handle to the timed out NAT test
+ * @param tc not used
+ */
+static void
+do_timeout (void *cls,
+                 const struct GNUNET_SCHEDULER_TaskContext * tc)
+{
+  struct GNUNET_NAT_Test *nh = (struct GNUNET_NAT_Test *) cls;
+  
+  nh->ttask = GNUNET_SCHEDULER_NO_TASK;
+  nh->report (nh->report_cls, GNUNET_NAT_ERROR_TIMEOUT);
+  
+  GNUNET_NAT_test_stop(nh);
+}
+
+
+/**
  * Start testing if NAT traversal works using the
  * given configuration (IPv4-only).
+ * 
+ * ALL failures are reported directly to the report callback
  *
  * @param cfg configuration for the NAT traversal
  * @param is_tcp #GNUNET_YES to test TCP, #GNUNET_NO to test UDP
  * @param bnd_port port to bind to, 0 for connection reversal
  * @param adv_port externally advertised port to use
+ * @param timeout delay after which the test should be aborted
  * @param report function to call with the result of the test
  * @param report_cls closure for @a report
  * @return handle to cancel NAT test
@@ -379,6 +407,7 @@ GNUNET_NAT_test_start (const struct GNUNET_CONFIGURATION_Handle *cfg,
                        int is_tcp,
                        uint16_t bnd_port,
                        uint16_t adv_port,
+                       struct GNUNET_TIME_Relative timeout,
                        GNUNET_NAT_TestCallback report,
                        void *report_cls)
 {
@@ -401,6 +430,7 @@ GNUNET_NAT_test_start (const struct GNUNET_CONFIGURATION_Handle *cfg,
   nh->adv_port = adv_port;
   nh->report = report;
   nh->report_cls = report_cls;
+  nh->ttask = GNUNET_SCHEDULER_NO_TASK;
   if (0 == bnd_port)
   {
     nh->nat =
@@ -459,6 +489,7 @@ GNUNET_NAT_test_start (const struct GNUNET_CONFIGURATION_Handle *cfg,
       return NULL;
     }
   }
+  nh->ttask = GNUNET_SCHEDULER_add_delayed (timeout, &do_timeout, nh);
   return nh;
 }
 
@@ -489,6 +520,8 @@ GNUNET_NAT_test_stop (struct GNUNET_NAT_Test *tst)
     GNUNET_NETWORK_socket_close (pos->sock);
     GNUNET_free (pos);
   }
+  if (GNUNET_SCHEDULER_NO_TASK != tst->ttask)
+    GNUNET_SCHEDULER_cancel (tst->ttask);
   if (GNUNET_SCHEDULER_NO_TASK != tst->ltask)
     GNUNET_SCHEDULER_cancel (tst->ltask);
   if (NULL != tst->lsock)
diff --git a/src/nat/test_nat_test.c b/src/nat/test_nat_test.c
index 39bd6612e..e8f0f5a47 100644
--- a/src/nat/test_nat_test.c
+++ b/src/nat/test_nat_test.c
@@ -39,23 +39,12 @@ static int ret = 1;
 
 static struct GNUNET_NAT_Test *tst;
 
-static GNUNET_SCHEDULER_TaskIdentifier end;
-
-static void
-end_test (void *cls, const struct GNUNET_SCHEDULER_TaskContext *tc)
-{
-  GNUNET_NAT_test_stop (tst);
-}
-
-
 static void
 report_success (void *cls,
                 enum GNUNET_NAT_FailureCode aret)
 {
   GNUNET_assert (GNUNET_NAT_ERROR_SUCCESS == aret);
   ret = 0;
-  GNUNET_SCHEDULER_cancel (end);
-  end = GNUNET_SCHEDULER_add_now (&end_test, NULL);
 }
 
 
@@ -67,12 +56,9 @@ run (void *cls, char *const *args, const char *cfgfile,
      const struct GNUNET_CONFIGURATION_Handle *cfg)
 {
   tst =
-      GNUNET_NAT_test_start (cfg, GNUNET_YES, 1285, 1285,
+      GNUNET_NAT_test_start (cfg, GNUNET_YES, 1285, 1285, TIMEOUT
                              &report_success,
                              NULL);
-  if (NULL == tst)
-    return;
-  end = GNUNET_SCHEDULER_add_delayed (TIMEOUT, &end_test, NULL);
 }