-modify NAT API to return error messages about problems detected

6 files changed, 230 insertions, 103 deletions

diff --git a/src/nat/nat.c b/src/nat/nat.c
index 69c60291c..ae40a2cd1 100644
--- a/src/nat/nat.c
+++ b/src/nat/nat.c
@@ -1011,12 +1011,14 @@ 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
  */
 static void
 upnp_add (void *cls,
           int add_remove,
           const struct sockaddr *addr,
-          socklen_t addrlen)
+          socklen_t addrlen,
+          const char *emsg)
 {
   struct GNUNET_NAT_Handle *h = cls;
   struct LocalAddressList *pos;
diff --git a/src/nat/nat_auto.c b/src/nat/nat_auto.c
index 9e7ff23f2..070099b23 100644
--- a/src/nat/nat_auto.c
+++ b/src/nat/nat_auto.c
@@ -185,9 +185,12 @@ fail_timeout (void *cls,
  *
  * @param cls the auto handle
  * @param success currently always #GNUNET_OK
+ * @param emsg NULL on success, otherwise an error message
  */
 static void
-result_callback (void *cls, int success)
+result_callback (void *cls,
+                 int success,
+                 const char *emsg)
 {
   struct GNUNET_NAT_AutoHandle *ah = cls;
 
@@ -250,10 +253,12 @@ test_online (struct GNUNET_NAT_AutoHandle *ah)
  *
  * @param cls closure with our setup context
  * @param addr the address, NULL on errors
+ * @param emsg NULL on success, otherwise an error message
  */
 static void
 set_external_ipv4 (void *cls,
-                   const struct in_addr *addr)
+                   const struct in_addr *addr,
+                   const char *emsg)
 {
   struct GNUNET_NAT_AutoHandle *ah = cls;
   char buf[INET_ADDRSTRLEN];
@@ -519,7 +524,8 @@ next_phase (struct GNUNET_NAT_AutoHandle *ah)
     diff = GNUNET_CONFIGURATION_get_diff (ah->initial_cfg,
                                           ah->cfg);
     ah->fin_cb (ah->fin_cb_cls,
-                diff);
+                diff,
+                NULL);
     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 497371e86..a5ae4ad08 100644
--- a/src/nat/nat_mini.c
+++ b/src/nat/nat_mini.c
@@ -1,6 +1,6 @@
 /*
      This file is part of GNUnet.
-     (C) 2011 Christian Grothoff (and other contributing authors)
+     (C) 2011-2014 Christian Grothoff (and other contributing authors)
 
      GNUnet is free software; you can redistribute it and/or modify
      it under the terms of the GNU General Public License as published
@@ -59,7 +59,7 @@ struct GNUNET_NAT_ExternalHandle
   GNUNET_NAT_IPCallback cb;
 
   /**
-   * Closure for 'cb'.
+   * Closure for @e cb.
    */
   void *cb_cls;
 
@@ -79,7 +79,7 @@ struct GNUNET_NAT_ExternalHandle
   struct GNUNET_DISK_PipeHandle *opipe;
 
   /**
-   * Read handle of 'opipe'.
+   * Read handle of @e opipe.
    */
   const struct GNUNET_DISK_FileHandle *r;
 
@@ -105,11 +105,12 @@ struct GNUNET_NAT_ExternalHandle
  * Read the output of 'external-ip' into buf.  When complete, parse the
  * address and call our callback.
  *
- * @param cls the 'struct GNUNET_NAT_ExternalHandle'
+ * @param cls the `struct GNUNET_NAT_ExternalHandle`
  * @param tc scheduler context
  */
 static void
-read_external_ipv4 (void *cls, const struct GNUNET_SCHEDULER_TaskContext *tc)
+read_external_ipv4 (void *cls,
+                    const struct GNUNET_SCHEDULER_TaskContext *tc)
 {
   struct GNUNET_NAT_ExternalHandle *eh = cls;
   ssize_t ret;
@@ -139,24 +140,50 @@ read_external_ipv4 (void *cls, const struct GNUNET_SCHEDULER_TaskContext *tc)
     eh->buf[eh->off - 1] = '\0';
     if (1 == inet_pton (AF_INET, eh->buf, &addr))
     {
-      if (addr.s_addr == 0)
+      if (0 == addr.s_addr)
         iret = GNUNET_NO;       /* got 0.0.0.0 */
       else
         iret = GNUNET_OK;
     }
   }
-  eh->cb (eh->cb_cls, (iret == GNUNET_OK) ? &addr : NULL);
+  eh->cb (eh->cb_cls,
+          (GNUNET_OK == iret)
+          ? &addr :
+          NULL,
+          (GNUNET_OK == iret)
+          ? NULL
+          : _("no valid address was returned by `external-ip'"));
   GNUNET_NAT_mini_get_external_ipv4_cancel (eh);
 }
 
 
 /**
+ * (Asynchronously) signal error invoking "external-ip" to client.
+ *
+ * @param cls the `struct GNUNET_NAT_ExternalHandle` (freed)
+ * @param tc scheduler context
+ */
+static void
+signal_external_ip_error (void *cls,
+                          const struct GNUNET_SCHEDULER_TaskContext *tc)
+{
+  struct GNUNET_NAT_ExternalHandle *eh = cls;
+
+  eh->task = GNUNET_SCHEDULER_NO_TASK;
+  eh->cb (eh->cb_cls,
+          NULL,
+          _("`external-ip' command not found"));
+  GNUNET_free (eh);
+}
+
+
+/**
  * Try to get the external IPv4 address of this peer.
  *
  * @param timeout when to fail
  * @param cb function to call with result
- * @param cb_cls closure for 'cb'
- * @return handle for cancellation (can only be used until 'cb' is called), NULL on error
+ * @param cb_cls closure for @a cb
+ * @return handle for cancellation (can only be used until @a cb is called), NULL on error
  */
 struct GNUNET_NAT_ExternalHandle *
 GNUNET_NAT_mini_get_external_ipv4 (struct GNUNET_TIME_Relative timeout,
@@ -164,37 +191,45 @@ GNUNET_NAT_mini_get_external_ipv4 (struct GNUNET_TIME_Relative timeout,
 {
   struct GNUNET_NAT_ExternalHandle *eh;
 
-  if (GNUNET_SYSERR == GNUNET_OS_check_helper_binary ("external-ip", GNUNET_NO, NULL))
+  eh = GNUNET_new (struct GNUNET_NAT_ExternalHandle);
+  eh->cb = cb;
+  eh->cb_cls = cb_cls;
+  if (GNUNET_SYSERR ==
+      GNUNET_OS_check_helper_binary ("external-ip", GNUNET_NO, NULL))
   {
     LOG (GNUNET_ERROR_TYPE_INFO,
          _("`external-ip' command not found\n"));
-    return NULL;
+    eh->task = GNUNET_SCHEDULER_add_now (&signal_external_ip_error,
+                                         eh);
+    return eh;
   }
   LOG (GNUNET_ERROR_TYPE_DEBUG,
        "Running `external-ip' to determine our external IP\n");
-  eh = GNUNET_new (struct GNUNET_NAT_ExternalHandle);
-  eh->cb = cb;
-  eh->cb_cls = cb_cls;
   eh->opipe = GNUNET_DISK_pipe (GNUNET_YES, GNUNET_YES, GNUNET_NO, GNUNET_YES);
   if (NULL == eh->opipe)
   {
-    GNUNET_free (eh);
-    return NULL;
+    eh->task = GNUNET_SCHEDULER_add_now (&signal_external_ip_error,
+                                         eh);
+    return eh;
   }
   eh->eip =
-    GNUNET_OS_start_process (GNUNET_NO, 0, NULL, eh->opipe, "external-ip", "external-ip",
+    GNUNET_OS_start_process (GNUNET_NO, 0, NULL, eh->opipe,
+                             "external-ip", "external-ip",
                                NULL);
   if (NULL == eh->eip)
   {
     GNUNET_DISK_pipe_close (eh->opipe);
-    GNUNET_free (eh);
-    return NULL;
+    eh->task = GNUNET_SCHEDULER_add_now (&signal_external_ip_error,
+                                         eh);
+    return eh;
   }
   GNUNET_DISK_pipe_close_end (eh->opipe, GNUNET_DISK_PIPE_END_WRITE);
   eh->timeout = GNUNET_TIME_relative_to_absolute (timeout);
   eh->r = GNUNET_DISK_pipe_handle (eh->opipe, GNUNET_DISK_PIPE_END_READ);
   eh->task =
-      GNUNET_SCHEDULER_add_read_file (timeout, eh->r, &read_external_ipv4, eh);
+      GNUNET_SCHEDULER_add_read_file (timeout,
+                                      eh->r,
+                                      &read_external_ipv4, eh);
   return eh;
 }
 
@@ -207,9 +242,13 @@ GNUNET_NAT_mini_get_external_ipv4 (struct GNUNET_TIME_Relative timeout,
 void
 GNUNET_NAT_mini_get_external_ipv4_cancel (struct GNUNET_NAT_ExternalHandle *eh)
 {
-  (void) GNUNET_OS_process_kill (eh->eip, SIGKILL);
-  GNUNET_OS_process_destroy (eh->eip);
-  GNUNET_DISK_pipe_close (eh->opipe);
+  if (NULL != eh->eip)
+  {
+    (void) GNUNET_OS_process_kill (eh->eip, SIGKILL);
+    GNUNET_OS_process_destroy (eh->eip);
+  }
+  if (NULL != eh->opipe)
+    GNUNET_DISK_pipe_close (eh->opipe);
   if (GNUNET_SCHEDULER_NO_TASK != eh->task)
     GNUNET_SCHEDULER_cancel (eh->task);
   GNUNET_free (eh);
@@ -225,10 +264,10 @@ struct GNUNET_NAT_MiniHandle
   /**
    * Function to call on mapping changes.
    */
-  GNUNET_NAT_AddressCallback ac;
+  GNUNET_NAT_MiniAddressCallback ac;
 
   /**
-   * Closure for 'ac'.
+   * Closure for @e ac.
    */
   void *ac_cls;
 
@@ -282,9 +321,9 @@ struct GNUNET_NAT_MiniHandle
 
 
 /**
- * Run upnpc -l to find out if our mapping changed.
+ * Run "upnpc -l" to find out if our mapping changed.
  *
- * @param cls the 'struct GNUNET_NAT_MiniHandle'
+ * @param cls the `struct GNUNET_NAT_MiniHandle`
  * @param tc scheduler context
  */
 static void
@@ -292,9 +331,9 @@ do_refresh (void *cls, const struct GNUNET_SCHEDULER_TaskContext *tc);
 
 
 /**
- * Process the output from the 'upnpc -r' command.
+ * Process the output from the "upnpc -r" command.
  *
- * @param cls the 'struct GNUNET_NAT_MiniHandle'
+ * @param cls the `struct GNUNET_NAT_MiniHandle`
  * @param line line of output, NULL at the end
  */
 static void
@@ -302,10 +341,39 @@ process_map_output (void *cls, const char *line);
 
 
 /**
- * Process the output from 'upnpc -l' to see if our
+ * Run "upnpc -r" to map our internal port.
+ *
+ * @param mini our handle
+ */
+static void
+run_upnpc_r (struct GNUNET_NAT_MiniHandle *mini)
+{
+  char pstr[6];
+
+  GNUNET_snprintf (pstr,
+                   sizeof (pstr),
+                   "%u",
+                   (unsigned int) mini->port);
+  mini->map_cmd =
+    GNUNET_OS_command_run (&process_map_output, mini, MAP_TIMEOUT,
+                           "upnpc", "upnpc", "-r", pstr,
+                           mini->is_tcp ? "tcp" : "udp", NULL);
+  if (NULL == mini->map_cmd)
+  {
+    mini->ac (mini->ac_cls,
+              GNUNET_SYSERR,
+              NULL, 0,
+              _("Failed to run `upnpc` command"));
+    return;
+  }
+}
+
+
+/**
+ * Process the output from "upnpc -l" to see if our
  * external mapping changed.  If so, do the notifications.
  *
- * @param cls the 'struct GNUNET_NAT_MiniHandle'
+ * @param cls the `struct GNUNET_NAT_MiniHandle`
  * @param line line of output, NULL at the end
  */
 static void
@@ -321,26 +389,20 @@ process_refresh_output (void *cls, const char *line)
   {
     GNUNET_OS_command_stop (mini->refresh_cmd);
     mini->refresh_cmd = NULL;
-    if (mini->found == GNUNET_NO)
+    if (GNUNET_NO == mini->found)
     {
       /* mapping disappeared, try to re-create */
-      if (mini->did_map)
+      if (GNUNET_YES == mini->did_map)
       {
-        mini->ac (mini->ac_cls, GNUNET_NO,
+        mini->ac (mini->ac_cls,
+                  GNUNET_NO,
                   (const struct sockaddr *) &mini->current_addr,
-                  sizeof (mini->current_addr));
+                  sizeof (mini->current_addr),
+                  NULL);
         mini->did_map = GNUNET_NO;
       }
-      GNUNET_snprintf (pstr, sizeof (pstr), "%u", (unsigned int) mini->port);
-      mini->map_cmd =
-          GNUNET_OS_command_run (&process_map_output, mini, MAP_TIMEOUT,
-                                 "upnpc", "upnpc", "-r", pstr,
-                                 mini->is_tcp ? "tcp" : "udp", NULL);
-      if (NULL != mini->map_cmd)
-        return;
+      run_upnpc_r (mini);
     }
-    mini->refresh_task =
-        GNUNET_SCHEDULER_add_delayed (MAP_REFRESH_FREQ, &do_refresh, mini);
     return;
   }
   if (!mini->did_map)
@@ -360,11 +422,13 @@ process_refresh_output (void *cls, const char *line)
     /* update mapping */
     mini->ac (mini->ac_cls, GNUNET_NO,
               (const struct sockaddr *) &mini->current_addr,
-              sizeof (mini->current_addr));
+              sizeof (mini->current_addr),
+              NULL);
     mini->current_addr.sin_addr = exip;
     mini->ac (mini->ac_cls, GNUNET_YES,
               (const struct sockaddr *) &mini->current_addr,
-              sizeof (mini->current_addr));
+              sizeof (mini->current_addr),
+              NULL);
     return;
   }
   /*
@@ -395,16 +459,18 @@ process_refresh_output (void *cls, const char *line)
   /* external port changed, update mapping */
   mini->ac (mini->ac_cls, GNUNET_NO,
             (const struct sockaddr *) &mini->current_addr,
-            sizeof (mini->current_addr));
+            sizeof (mini->current_addr),
+            NULL);
   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));
+            sizeof (mini->current_addr),
+            NULL);
 }
 
 
 /**
- * Run upnpc -l to find out if our mapping changed.
+ * Run "upnpc -l" to find out if our mapping changed.
  *
  * @param cls the 'struct GNUNET_NAT_MiniHandle'
  * @param tc scheduler context
@@ -413,25 +479,49 @@ static void
 do_refresh (void *cls, const struct GNUNET_SCHEDULER_TaskContext *tc)
 {
   struct GNUNET_NAT_MiniHandle *mini = cls;
+  int ac;
 
+  mini->refresh_task =
+    GNUNET_SCHEDULER_add_delayed (MAP_REFRESH_FREQ,
+                                  &do_refresh, mini);
   LOG (GNUNET_ERROR_TYPE_DEBUG,
        "Running `upnpc' to check if our mapping still exists\n");
-  mini->refresh_task = GNUNET_SCHEDULER_NO_TASK;
   mini->found = GNUNET_NO;
+  ac = GNUNET_NO;
+  if (NULL != mini->map_cmd)
+  {
+    /* took way too long, abort it! */
+    GNUNET_OS_command_stop (mini->map_cmd);
+    mini->map_cmd = NULL;
+    ac = GNUNET_YES;
+  }
+  if (NULL != mini->refresh_cmd)
+  {
+    /* took way too long, abort it! */
+    GNUNET_OS_command_stop (mini->refresh_cmd);
+    mini->refresh_cmd = NULL;
+    ac = GNUNET_YES;
+  }
   mini->refresh_cmd =
       GNUNET_OS_command_run (&process_refresh_output, mini, MAP_TIMEOUT,
                              "upnpc", "upnpc", "-l", NULL);
+  if (GNUNET_YES == ac)
+    mini->ac (mini->ac_cls,
+              GNUNET_SYSERR,
+              NULL, 0,
+              _("`upnpc' command took too long, process killed"));
 }
 
 
 /**
  * Process the output from the 'upnpc -r' command.
  *
- * @param cls the 'struct GNUNET_NAT_MiniHandle'
+ * @param cls the `struct GNUNET_NAT_MiniHandle`
  * @param line line of output, NULL at the end
  */
 static void
-process_map_output (void *cls, const char *line)
+process_map_output (void *cls,
+                    const char *line)
 {
   struct GNUNET_NAT_MiniHandle *mini = cls;
   const char *ipaddr;
@@ -443,7 +533,13 @@ process_map_output (void *cls, const char *line)
   {
     GNUNET_OS_command_stop (mini->map_cmd);
     mini->map_cmd = NULL;
-    mini->refresh_task =
+    if (GNUNET_YES != mini->did_map)
+      mini->ac (mini->ac_cls,
+                GNUNET_SYSERR,
+                NULL, 0,
+                _("`upnpc' command failed to establish port mapping"));
+    if (GNUNET_SCHEDULER_NO_TASK == mini->refresh_task)
+      mini->refresh_task =
         GNUNET_SCHEDULER_add_delayed (MAP_REFRESH_FREQ, &do_refresh, mini);
     return;
   }
@@ -475,34 +571,41 @@ process_map_output (void *cls, const char *line)
   mini->did_map = GNUNET_YES;
   mini->ac (mini->ac_cls, GNUNET_YES,
             (const struct sockaddr *) &mini->current_addr,
-            sizeof (mini->current_addr));
+            sizeof (mini->current_addr),
+            NULL);
 }
 
 
 /**
  * Start mapping the given port using (mini)upnpc.  This function
  * should typically not be used directly (it is used within the
- * general-purpose 'GNUNET_NAT_register' code).  However, it can be
+ * general-purpose #GNUNET_NAT_register() code).  However, it can be
  * used if specifically UPnP-based NAT traversal is to be used or
  * tested.
  *
  * @param port port to map
- * @param is_tcp GNUNET_YES to map TCP, GNUNET_NO for UDP
+ * @param is_tcp #GNUNET_YES to map TCP, #GNUNET_NO for UDP
  * @param ac function to call with mapping result
- * @param ac_cls closure for 'ac'
+ * @param ac_cls closure for @a ac
  * @return NULL on error (no 'upnpc' installed)
  */
 struct GNUNET_NAT_MiniHandle *
-GNUNET_NAT_mini_map_start (uint16_t port, int is_tcp,
-                           GNUNET_NAT_AddressCallback ac, void *ac_cls)
+GNUNET_NAT_mini_map_start (uint16_t port,
+                           int is_tcp,
+                           GNUNET_NAT_MiniAddressCallback ac,
+                           void *ac_cls)
 {
   struct GNUNET_NAT_MiniHandle *ret;
-  char pstr[6];
 
-  if (GNUNET_SYSERR == GNUNET_OS_check_helper_binary ("upnpc", GNUNET_NO, NULL))
+  if (GNUNET_SYSERR ==
+      GNUNET_OS_check_helper_binary ("upnpc", GNUNET_NO, NULL))
   {
     LOG (GNUNET_ERROR_TYPE_INFO,
          _("`upnpc' command not found\n"));
+    ac (ac_cls,
+        GNUNET_SYSERR,
+        NULL, 0,
+        _("`upnpc` command not found"));
     return NULL;
   }
   LOG (GNUNET_ERROR_TYPE_DEBUG,
@@ -512,15 +615,9 @@ GNUNET_NAT_mini_map_start (uint16_t port, int is_tcp,
   ret->ac_cls = ac_cls;
   ret->is_tcp = is_tcp;
   ret->port = port;
-  GNUNET_snprintf (pstr, sizeof (pstr), "%u", (unsigned int) port);
-  ret->map_cmd =
-      GNUNET_OS_command_run (&process_map_output, ret, MAP_TIMEOUT, "upnpc",
-                             "upnpc", "-r", pstr, is_tcp ? "tcp" : "udp", NULL);
-  if (NULL != ret->map_cmd)
-    return ret;
   ret->refresh_task =
-      GNUNET_SCHEDULER_add_delayed (MAP_REFRESH_FREQ, &do_refresh, ret);
-
+    GNUNET_SCHEDULER_add_delayed (MAP_REFRESH_FREQ, &do_refresh, ret);
+  run_upnpc_r (ret);
   return ret;
 }
 
@@ -538,7 +635,8 @@ process_unmap_output (void *cls, const char *line)
 
   if (NULL == line)
   {
-    LOG (GNUNET_ERROR_TYPE_DEBUG, "UPnP unmap done\n");
+    LOG (GNUNET_ERROR_TYPE_DEBUG,
+         "UPnP unmap done\n");
     GNUNET_OS_command_stop (mini->unmap_cmd);
     mini->unmap_cmd = NULL;
     GNUNET_free (mini);
@@ -566,14 +664,14 @@ GNUNET_NAT_mini_map_stop (struct GNUNET_NAT_MiniHandle *mini)
     GNUNET_SCHEDULER_cancel (mini->refresh_task);
     mini->refresh_task = GNUNET_SCHEDULER_NO_TASK;
   }
-  if (mini->refresh_cmd != NULL)
+  if (NULL != mini->refresh_cmd)
   {
     GNUNET_OS_command_stop (mini->refresh_cmd);
     mini->refresh_cmd = NULL;
   }
-  if (!mini->did_map)
+  if (GNUNET_NO == mini->did_map)
   {
-    if (mini->map_cmd != NULL)
+    if (NULL != mini->map_cmd)
     {
       GNUNET_OS_command_stop (mini->map_cmd);
       mini->map_cmd = NULL;
@@ -583,13 +681,17 @@ 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));
+            sizeof (mini->current_addr),
+            NULL);
   /* 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... */
-  GNUNET_snprintf (pstr, sizeof (pstr), "%u",
+  GNUNET_snprintf (pstr,
+                   sizeof (pstr),
+                   "%u",
                    (unsigned int) ntohs (mini->current_addr.sin_port));
-  LOG (GNUNET_ERROR_TYPE_DEBUG, "Unmapping port %u with UPnP\n",
+  LOG (GNUNET_ERROR_TYPE_DEBUG,
+       "Unmapping port %u with UPnP\n",
        ntohs (mini->current_addr.sin_port));
   mini->unmap_cmd =
       GNUNET_OS_command_run (&process_unmap_output, mini, UNMAP_TIMEOUT,
diff --git a/src/nat/nat_test.c b/src/nat/nat_test.c
index c7de96d2f..f3b2b41ef 100644
--- a/src/nat/nat_test.c
+++ b/src/nat/nat_test.c
@@ -177,7 +177,7 @@ reversal_cb (void *cls,
   struct GNUNET_NAT_Test *h = cls;
   const struct sockaddr_in *sa;
 
-  if (addrlen != sizeof (struct sockaddr_in))
+  if (sizeof (struct sockaddr_in) != addrlen)
     return;
   sa = (const struct sockaddr_in *) addr;
   if (h->data != sa->sin_port)
@@ -187,7 +187,7 @@ reversal_cb (void *cls,
     return;                     /* wrong port */
   }
   /* report success */
-  h->report (h->report_cls, GNUNET_OK);
+  h->report (h->report_cls, GNUNET_OK, NULL);
 }
 
 
@@ -199,7 +199,8 @@ reversal_cb (void *cls,
  * @param tc scheduler context
  */
 static void
-do_udp_read (void *cls, const struct GNUNET_SCHEDULER_TaskContext *tc)
+do_udp_read (void *cls,
+             const struct GNUNET_SCHEDULER_TaskContext *tc)
 {
   struct GNUNET_NAT_Test *tst = cls;
   uint16_t data;
@@ -214,7 +215,7 @@ do_udp_read (void *cls, const struct GNUNET_SCHEDULER_TaskContext *tc)
        GNUNET_NETWORK_socket_recv (tst->lsock, &data, sizeof (data))))
   {
     if (data == tst->data)
-      tst->report (tst->report_cls, GNUNET_OK);
+      tst->report (tst->report_cls, GNUNET_OK, NULL);
     else
       LOG (GNUNET_ERROR_TYPE_DEBUG,
            "Received data mismatches expected value\n");
@@ -249,10 +250,10 @@ do_read (void *cls,
        GNUNET_NETWORK_socket_recv (na->sock, &data, sizeof (data))))
   {
     if (data == tst->data)
-      tst->report (tst->report_cls, GNUNET_OK);
+      tst->report (tst->report_cls, GNUNET_OK, NULL);
     else
       LOG (GNUNET_ERROR_TYPE_DEBUG,
-           "Received data mismatches expected value\n");
+           "Received data does not match expected value\n");
   }
   else
     LOG (GNUNET_ERROR_TYPE_DEBUG,
@@ -295,8 +296,9 @@ do_accept (void *cls,
   wl->sock = s;
   wl->h = tst;
   wl->rtask =
-      GNUNET_SCHEDULER_add_read_net (GNUNET_TIME_UNIT_FOREVER_REL, wl->sock,
-                                     &do_read, wl);
+    GNUNET_SCHEDULER_add_read_net (GNUNET_TIME_UNIT_FOREVER_REL,
+                                   wl->sock,
+                                   &do_read, wl);
   GNUNET_CONTAINER_DLL_insert (tst->na_head, tst->na_tail, wl);
 }
 
@@ -331,7 +333,8 @@ addr_cb (void *cls,
          GNUNET_a2s (addr, addrlen));
     return;                     /* ignore IPv6 here */
   }
-  LOG (GNUNET_ERROR_TYPE_DEBUG, "Asking gnunet-nat-server to connect to `%s'\n",
+  LOG (GNUNET_ERROR_TYPE_DEBUG,
+       "Asking gnunet-nat-server to connect to `%s'\n",
        GNUNET_a2s (addr, addrlen));
   sa = (const struct sockaddr_in *) addr;
   msg.header.size = htons (sizeof (struct GNUNET_NAT_TestMessage));
@@ -398,7 +401,7 @@ GNUNET_NAT_test_start (const struct GNUNET_CONFIGURATION_Handle *cfg,
   ret->adv_port = adv_port;
   ret->report = report;
   ret->report_cls = report_cls;
-  if (bnd_port == 0)
+  if (0 == bnd_port)
   {
     ret->nat =
         GNUNET_NAT_register (cfg, is_tcp, 0, 0, NULL, NULL, &addr_cb,
diff --git a/src/nat/test_nat_mini.c b/src/nat/test_nat_mini.c
index c503a837a..6fe4fd5b9 100644
--- a/src/nat/test_nat_mini.c
+++ b/src/nat/test_nat_mini.c
@@ -42,12 +42,17 @@
  * believes to be valid for the transport.
  */
 static void
-addr_callback (void *cls, int add_remove, const struct sockaddr *addr,
-               socklen_t addrlen)
+addr_callback (void *cls, int add_remove,
+               const struct sockaddr *addr,
+               socklen_t addrlen,
+               const char *emsg)
 {
-  fprintf (stderr, "Address changed: %s `%s' (%u bytes)\n",
-           add_remove == GNUNET_YES ? "added" : "removed", GNUNET_a2s (addr,
-                                                                       addrlen),
+  fprintf (stderr,
+           "Address changed: %s `%s' (%u bytes)\n",
+           add_remove == GNUNET_YES
+           ? "added" : "removed",
+           GNUNET_a2s (addr,
+                       addrlen),
            (unsigned int) addrlen);
 }
 
@@ -76,7 +81,8 @@ run (void *cls, char *const *args, const char *cfgfile,
   struct GNUNET_NAT_MiniHandle *mini;
 
   GNUNET_log (GNUNET_ERROR_TYPE_INFO,
-              "Requesting NAT redirection for port %u...\n", PORT);
+              "Requesting NAT redirection for port %u...\n",
+              PORT);
   mini = GNUNET_NAT_mini_map_start (PORT, GNUNET_YES /* tcp */ ,
                                     &addr_callback, NULL);
   if (NULL == mini)
diff --git a/src/nat/test_nat_test.c b/src/nat/test_nat_test.c
index 752c8f145..64425ab47 100644
--- a/src/nat/test_nat_test.c
+++ b/src/nat/test_nat_test.c
@@ -47,8 +47,11 @@ end_test (void *cls, const struct GNUNET_SCHEDULER_TaskContext *tc)
   GNUNET_NAT_test_stop (tst);
 }
 
+
 static void
-report_success (void *cls, int success)
+report_success (void *cls,
+                int success,
+                const char *emsg)
 {
   GNUNET_assert (GNUNET_OK == success);
   ret = 0;
@@ -56,6 +59,7 @@ report_success (void *cls, int success)
   end = GNUNET_SCHEDULER_add_now (&end_test, NULL);
 }
 
+
 /**
  * Main function run with scheduler.
  */
@@ -64,7 +68,8 @@ 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, &report_success,
+      GNUNET_NAT_test_start (cfg, GNUNET_YES, 1285, 1285,
+                             &report_success,
                              NULL);
   if (NULL == tst)
     return;
@@ -100,10 +105,12 @@ main (int argc, char *const argv[])
     return 0;
   }
 
-  gns =
-      GNUNET_OS_start_process (GNUNET_YES, GNUNET_OS_INHERIT_STD_OUT_AND_ERR, NULL, NULL, "gnunet-nat-server",
-                               "gnunet-nat-server",
-                               "-c", "test_nat_test_data.conf", "12345", NULL);
+  gns = GNUNET_OS_start_process (GNUNET_YES,
+                                 GNUNET_OS_INHERIT_STD_OUT_AND_ERR,
+                                 NULL, NULL, "gnunet-nat-server",
+                                 "gnunet-nat-server",
+                                 "-c", "test_nat_test_data.conf",
+                                 "12345", NULL);
   GNUNET_assert (NULL != gns);
   GNUNET_PROGRAM_run (3, argv_prog, "test-nat-test", "nohelp", options, &run,
                       NULL);
@@ -111,7 +118,8 @@ main (int argc, char *const argv[])
   GNUNET_break (GNUNET_OK == GNUNET_OS_process_wait (gns));
   GNUNET_OS_process_destroy (gns);
   if (0 != ret)
-    fprintf (stderr, "NAT test failed to report success\n");
+    fprintf (stderr,
+             "NAT test failed to report success\n");
   return ret;
 }