Fix review,

- Many fixes.
- Do not mark test as failed if no connection
 PATCH 2/3

4 files changed, 145 insertions, 101 deletions

diff --git a/src/nat/nat_auto.c b/src/nat/nat_auto.c
index dc7eecb3e..4701d744e 100644
--- a/src/nat/nat_auto.c
+++ b/src/nat/nat_auto.c
@@ -243,7 +243,7 @@ stop_stun ()
     GNUNET_SCHEDULER_cancel (ltask4);
 
   //Clean socket
-  if(NULL != ltask4)
+  if(NULL != lsock4)
     GNUNET_NETWORK_socket_close (lsock4);
 
 }
diff --git a/src/nat/nat_stun.c b/src/nat/nat_stun.c
index 57b25e20a..7b41b3387 100644
--- a/src/nat/nat_stun.c
+++ b/src/nat/nat_stun.c
@@ -82,7 +82,7 @@ struct GNUNET_NAT_STUN_Handle {
     /**
      * Function to call when a error occours
     */
-    GNUNET_NAT_stun_RequestCallback cb;
+    GNUNET_NAT_STUN_ErrorCallback cb;
 
     /**
      * Closure for @e cb.
@@ -441,7 +441,10 @@ GNUNET_NAT_stun_handle_packet(const void *data, size_t len, struct sockaddr_in *
 static void
 clean(struct GNUNET_NAT_STUN_Handle * handle)
 {
+  if(handle->stun_server)
+  {
     GNUNET_free(handle->stun_server);
+  }
     GNUNET_free(handle);
 
 }
@@ -485,7 +488,7 @@ stun_dns_callback (void *cls,
 
         if( GNUNET_NO == request->dns_success){
             LOG (GNUNET_ERROR_TYPE_INFO, "Error resolving host %s\n", request->stun_server);
-            request->cb(request->cb_cls, GNUNET_NAT_ERROR_INTERNAL_NETWORK_ERROR);
+            request->cb(request->cb_cls, GNUNET_NAT_ERROR_NOT_ONLINE);
             clean(request);
 
         }
@@ -536,11 +539,14 @@ stun_dns_callback (void *cls,
  * @param server, the address of the stun server
  * @param port, port of the stun server
  * @param sock the socket used to send the request
+ * @param cb, callback in case of error
  * @return #GNUNET_OK success, #GNUNET_NO on error.
  */
 int
-GNUNET_NAT_stun_make_request(char * server, int port,
-                             struct GNUNET_NETWORK_Handle * sock,GNUNET_NAT_stun_RequestCallback cb,
+GNUNET_NAT_stun_make_request(char * server,
+                             int port,
+                             struct GNUNET_NETWORK_Handle * sock,
+                             GNUNET_NAT_STUN_ErrorCallback cb,
                              void *cb_cls)
 {
 
diff --git a/src/nat/test_stun.c b/src/nat/test_stun.c
index f14661969..edb7a89eb 100644
--- a/src/nat/test_stun.c
+++ b/src/nat/test_stun.c
@@ -75,6 +75,32 @@ print_answer(struct sockaddr_in* answer)
 }
 
 
+
+/**
+ * Function that terminates the test.
+ */
+static void
+stop ()
+{
+  GNUNET_log (GNUNET_ERROR_TYPE_INFO, "Stopping NAT and quitting...\n");
+
+  //Clean task
+  if(NULL != ltask4)
+  {
+    GNUNET_SCHEDULER_cancel (ltask4);
+    ltask4 = NULL;
+  }
+
+  //Clean socket
+  if(NULL != lsock4)
+  {
+    GNUNET_NETWORK_socket_close(lsock4);
+    lsock4 = NULL;
+  }
+
+}
+
+
 /**
  * Activity on our incoming socket.  Read data from the
  * incoming connection.
@@ -91,7 +117,7 @@ do_udp_read (void *cls,
         ssize_t rlen;
         struct sockaddr_in answer;
 
-    if ((0 != (tc->reason & GNUNET_SCHEDULER_REASON_READ_READY)) &&
+  if ((0 != (tc->reason & GNUNET_SCHEDULER_REASON_READ_READY)) &&
       (GNUNET_NETWORK_fdset_isset (tc->read_ready,
                                    lsock4)))
         {
@@ -100,16 +126,34 @@ do_udp_read (void *cls,
                 
                 //Lets handle the packet
                 memset(&answer, 0, sizeof(struct sockaddr_in));
-        GNUNET_NAT_stun_handle_packet(reply_buf,rlen, &answer);
 
-                //Print the answer
-                ret = 0;
-                print_answer(&answer);
+    if(GNUNET_OK == GNUNET_NAT_stun_handle_packet(reply_buf, rlen, &answer))
+    {
+      //Print the answer
+      ret = 0;
+      print_answer(&answer);
+
+      //Destroy the connection
+      GNUNET_NETWORK_socket_close(lsock4);
+      lsock4 = NULL;
+
+    }
+    else
+    {
+      //Lets try again, its a invalid message
+      ltask4 = GNUNET_SCHEDULER_add_read_net (TIMEOUT,
+                                              lsock4, &do_udp_read, NULL);
+    }
 
-        //Destroy the connection
-        GNUNET_NETWORK_socket_close(lsock4);
-                
         }
+  else
+  {
+    //We got a timeout
+    ltask4 = NULL;
+    stop();
+  }
+
+  ltask4 = NULL;
 
 
 }
@@ -150,30 +194,24 @@ bind_v4 ()
     return ls;
 }
 
-/**
- * Function that terminates the test.
- */
-static void
-stop ()
-{
-    GNUNET_log (GNUNET_ERROR_TYPE_INFO, "Stopping NAT and quitting...\n");
-
-    //Clean task
-    if(NULL != ltask4)
-        GNUNET_SCHEDULER_cancel (ltask4);
-
-    //Clean socket
-    if(NULL != ltask4)
-        GNUNET_NETWORK_socket_close (lsock4);
-
-}
 
 
 static void request_callback(void *cls,
-enum GNUNET_NAT_StatusCode result)
+enum GNUNET_NAT_StatusCode error)
 {
-    ret = result;
-    stop();
+  if(error == GNUNET_NAT_ERROR_NOT_ONLINE)
+  {
+    //If we are not online, mark the test as success
+    ret = 0;
+    GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+                "test-stun detected as offline, cant make STUN request.\n"
+                );
+  }
+  else
+  {
+    ret = error;
+  }
+  stop();
 };
 
 
@@ -185,30 +223,29 @@ run (void *cls, char *const *args, const char *cfgfile,
      const struct GNUNET_CONFIGURATION_Handle *cfg)
 {
 
-
-    //Lets create the socket
-    lsock4 = bind_v4 ();
-    if (NULL == lsock4)
-    {
-        GNUNET_log_strerror (GNUNET_ERROR_TYPE_ERROR, "bind");
-        GNUNET_SCHEDULER_shutdown ();
-        return;
-    }
-    else
-    {
-        //Lets call our function now when it accepts
-        ltask4 = GNUNET_SCHEDULER_add_read_net (GNUNET_TIME_UNIT_FOREVER_REL,
-                                                lsock4, &do_udp_read, NULL);
-        /* So you read once and what will happen if you get an irregular message? Repeat and add timeout */
-
-    }
-
-    GNUNET_log (GNUNET_ERROR_TYPE_INFO,
-                "Service listens on port %u\n",
-                port);
-    GNUNET_NAT_stun_make_request(stun_server, stun_port, lsock4, &request_callback, NULL);
-
-    //GNUNET_SCHEDULER_add_delayed (TIMEOUT, &stop, NULL);
+  //Lets create the socket
+  lsock4 = bind_v4 ();
+  ltask4 = NULL;
+  if (NULL == lsock4)
+  {
+      GNUNET_log_strerror (GNUNET_ERROR_TYPE_ERROR, "bind");
+      GNUNET_SCHEDULER_shutdown ();
+      return;
+  }
+  else
+  {
+      //Lets call our function now when it accepts
+      ltask4 = GNUNET_SCHEDULER_add_read_net (TIMEOUT,
+                                              lsock4, &do_udp_read, NULL );
+
+  }
+
+  GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+              "Service listens on port %u\n",
+              port);
+  GNUNET_NAT_stun_make_request(stun_server, stun_port, lsock4, &request_callback, NULL);
+
+  GNUNET_SCHEDULER_add_delayed (TIMEOUT, &stop, NULL);
 
 }
 
@@ -216,51 +253,52 @@ run (void *cls, char *const *args, const char *cfgfile,
 int
 main (int argc, char *const argv[])
 {
-    struct GNUNET_GETOPT_CommandLineOption options[] = {
-        GNUNET_GETOPT_OPTION_END
-    };
-
-    char *const argv_prog[] = {
-        "test-stun",
-        "-c",
-        "test_stun.conf",
-        NULL
-    };
-    GNUNET_log_setup ("test-stun",
-                      "WARNING",
-                      NULL);
-
-    /* Lets start resolver */
-    char *fn;
-    struct GNUNET_OS_Process *proc;
-
-    fn = GNUNET_OS_get_libexec_binary_path ("gnunet-service-resolver");
-    proc = GNUNET_OS_start_process (GNUNET_YES,
-                                    GNUNET_OS_INHERIT_STD_OUT_AND_ERR,
-                                    NULL, NULL, NULL,
-                                    fn,
-                                    "gnunet-service-resolver",
-                                    "-c", "test_stun.conf", NULL);
-
-    if (NULL != proc)
-    {
-      GNUNET_log (GNUNET_ERROR_TYPE_INFO, "This test was unable to start gnunet-service-resolver, and it is required to run ...\n");
-      exit(1);
-    }
+  struct GNUNET_GETOPT_CommandLineOption options[] = {
+      GNUNET_GETOPT_OPTION_END
+  };
+
+  char *const argv_prog[] = {
+      "test-stun",
+      "-c",
+      "test_stun.conf",
+      NULL
+  };
+  GNUNET_log_setup ("test-stun",
+                    "WARNING",
+                    NULL);
+
+  /* Lets start resolver */
+  char *fn;
+  struct GNUNET_OS_Process *proc;
+
+  fn = GNUNET_OS_get_libexec_binary_path ("gnunet-service-resolver");
+  proc = GNUNET_OS_start_process (GNUNET_YES,
+                                  GNUNET_OS_INHERIT_STD_OUT_AND_ERR,
+                                  NULL, NULL, NULL,
+                                  fn,
+                                  "gnunet-service-resolver",
+                                  "-c", "test_stun.conf", NULL);
+
+  if (NULL == proc)
+  {
+    GNUNET_log (GNUNET_ERROR_TYPE_INFO, "This test was unable to start gnunet-service-resolver, and it is required to run ...\n");
+    exit(1);
+  }
+
+  GNUNET_PROGRAM_run (3, argv_prog, "test-stun", "nohelp", options, &run, NULL);
+
+
+  /* Now kill the resolver */
+  if (0 != GNUNET_OS_process_kill (proc, GNUNET_TERM_SIG))
+  {
+      GNUNET_log_strerror (GNUNET_ERROR_TYPE_WARNING, "kill");
+  }
+  GNUNET_OS_process_wait (proc);
+  GNUNET_OS_process_destroy (proc);
+  proc = NULL;
+  GNUNET_free (fn);
 
-    GNUNET_PROGRAM_run (3, argv_prog, "test-stun", "nohelp", options, &run, NULL);
-
-    /* Now kill the resolver */
-    if (0 != GNUNET_OS_process_kill (proc, GNUNET_TERM_SIG))
-    {
-        GNUNET_log_strerror (GNUNET_ERROR_TYPE_WARNING, "kill");
-    }
-    GNUNET_OS_process_wait (proc);
-    GNUNET_OS_process_destroy (proc);
-    proc = NULL;
-    GNUNET_free (fn);
 
-    
         return ret;
 }
 
diff --git a/src/nat/test_stun.conf b/src/nat/test_stun.conf
index 42b36d66a..c8104f0a7 100644
--- a/src/nat/test_stun.conf
+++ b/src/nat/test_stun.conf
@@ -1,5 +1,5 @@
 [PATHS]
-GNUNET_TEST_HOME = /tmp/test-gnunetd-statistics/
+GNUNET_TEST_HOME = /tmp/test-stun
 
 [resolver]
 PORT = 22354