-bringing copyright tags up to FSF standard
[oweals/gnunet.git] / src / nat / nat_mini.c
index c40231067210a264faa3b7dae4430c7b69c54cff..17d9f62eed8e63d460f6e44a3d09094746aacf08 100644 (file)
@@ -1,6 +1,6 @@
 /*
      This file is part of GNUnet.
-     (C) 2011 Christian Grothoff (and other contributing authors)
+     Copyright (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
@@ -28,7 +28,7 @@
 #include "gnunet_nat_lib.h"
 #include "nat.h"
 
-#define DEBUG_NAT GNUNET_NO
+#define LOG(kind,...) GNUNET_log_from (kind, "nat", __VA_ARGS__)
 
 /**
  * How long do we give upnpc to create a mapping?
@@ -59,14 +59,14 @@ struct GNUNET_NAT_ExternalHandle
   GNUNET_NAT_IPCallback cb;
 
   /**
-   * Closure for 'cb'.
+   * Closure for @e cb.
    */
   void *cb_cls;
 
   /**
    * Read task.
    */
-  GNUNET_SCHEDULER_TaskIdentifier task;
+  struct GNUNET_SCHEDULER_Task * task;
 
   /**
    * Handle to 'external-ip' process.
@@ -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;
 
@@ -98,6 +98,10 @@ struct GNUNET_NAT_ExternalHandle
    */
   char buf[17];
 
+  /**
+   * Error code for better debugging and user feedback
+   */
+  enum GNUNET_NAT_StatusCode ret;
 };
 
 
@@ -105,23 +109,26 @@ 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;
   struct in_addr addr;
-  int iret;
 
-  eh->task = GNUNET_SCHEDULER_NO_TASK;
+  eh->task = NULL;
   if (GNUNET_YES == GNUNET_NETWORK_fdset_handle_isset (tc->read_ready, eh->r))
-    ret = GNUNET_DISK_file_read (eh->r,
-                                 &eh->buf[eh->off], sizeof (eh->buf) - eh->off);
-  else
+    ret =
+        GNUNET_DISK_file_read (eh->r, &eh->buf[eh->off],
+                               sizeof (eh->buf) - eh->off);
+  else {
+    eh->ret = GNUNET_NAT_ERROR_IPC_FAILURE;
     ret = -1;                   /* error reading, timeout, etc. */
+  }
   if (ret > 0)
   {
     /* try to read more */
@@ -132,30 +139,52 @@ read_external_ipv4 (void *cls, const struct GNUNET_SCHEDULER_TaskContext *tc)
                                         &read_external_ipv4, eh);
     return;
   }
-  iret = GNUNET_NO;
+  eh->ret = GNUNET_NAT_ERROR_EXTERNAL_IP_UTILITY_OUTPUT_INVALID;
   if ((eh->off > 7) && (eh->buf[eh->off - 1] == '\n'))
   {
     eh->buf[eh->off - 1] = '\0';
     if (1 == inet_pton (AF_INET, eh->buf, &addr))
     {
-      if (addr.s_addr == 0)
-        iret = GNUNET_NO;       /* got 0.0.0.0 */
+      if (0 != addr.s_addr)
+        eh->ret = GNUNET_NAT_ERROR_EXTERNAL_IP_ADDRESS_INVALID;       /* got 0.0.0.0 */
       else
-        iret = GNUNET_OK;
+        eh->ret = GNUNET_NAT_ERROR_SUCCESS;
     }
   }
-  eh->cb (eh->cb_cls, (iret == GNUNET_OK) ? &addr : NULL);
+  eh->cb (eh->cb_cls,
+          (GNUNET_NAT_ERROR_SUCCESS == eh->ret) ? &addr : NULL,
+          eh->ret);
   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 = NULL;
+  eh->cb (eh->cb_cls,
+          NULL,
+          eh->ret);
+  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), never NULL
  */
 struct GNUNET_NAT_ExternalHandle *
 GNUNET_NAT_mini_get_external_ipv4 (struct GNUNET_TIME_Relative timeout,
@@ -163,29 +192,49 @@ GNUNET_NAT_mini_get_external_ipv4 (struct GNUNET_TIME_Relative timeout,
 {
   struct GNUNET_NAT_ExternalHandle *eh;
 
-  eh = GNUNET_malloc (sizeof (struct GNUNET_NAT_ExternalHandle));
+  eh = GNUNET_new (struct GNUNET_NAT_ExternalHandle);
   eh->cb = cb;
   eh->cb_cls = cb_cls;
-  eh->opipe = GNUNET_DISK_pipe (GNUNET_YES, GNUNET_NO, GNUNET_YES);
+  eh->ret = GNUNET_NAT_ERROR_SUCCESS;
+  if (GNUNET_SYSERR ==
+      GNUNET_OS_check_helper_binary ("external-ip", GNUNET_NO, NULL))
+  {
+    LOG (GNUNET_ERROR_TYPE_INFO,
+        _("`external-ip' command not found\n"));
+    eh->ret = GNUNET_NAT_ERROR_EXTERNAL_IP_UTILITY_NOT_FOUND;
+    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->opipe = GNUNET_DISK_pipe (GNUNET_YES, GNUNET_YES, GNUNET_NO, GNUNET_YES);
   if (NULL == eh->opipe)
   {
-    GNUNET_free (eh);
-    return NULL;
+    eh->ret = GNUNET_NAT_ERROR_IPC_FAILURE;
+    eh->task = GNUNET_SCHEDULER_add_now (&signal_external_ip_error,
+                                         eh);
+    return eh;
   }
-  eh->eip = GNUNET_OS_start_process (NULL,
-                                     eh->opipe,
-                                     "external-ip", "external-ip", NULL);
+  eh->eip =
+      GNUNET_OS_start_process (GNUNET_NO, 0, NULL, eh->opipe, NULL,
+                               "external-ip", "external-ip",
+                               NULL);
   if (NULL == eh->eip)
   {
     GNUNET_DISK_pipe_close (eh->opipe);
-    GNUNET_free (eh);
-    return NULL;
+    eh->ret = GNUNET_NAT_ERROR_EXTERNAL_IP_UTILITY_FAILED;
+    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);
+  eh->task =
+      GNUNET_SCHEDULER_add_read_file (timeout,
+                                      eh->r,
+                                      &read_external_ipv4, eh);
   return eh;
 }
 
@@ -198,10 +247,14 @@ 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_close (eh->eip);
-  GNUNET_DISK_pipe_close (eh->opipe);
-  if (GNUNET_SCHEDULER_NO_TASK != eh->task)
+  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 (NULL != eh->task)
     GNUNET_SCHEDULER_cancel (eh->task);
   GNUNET_free (eh);
 }
@@ -216,10 +269,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;
 
@@ -247,7 +300,7 @@ struct GNUNET_NAT_MiniHandle
    * We check the mapping periodically to see if it
    * still works.  This task triggers the check.
    */
-  GNUNET_SCHEDULER_TaskIdentifier refresh_task;
+  struct GNUNET_SCHEDULER_Task * refresh_task;
 
   /**
    * Are we mapping TCP or UDP?
@@ -273,9 +326,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
@@ -283,19 +336,49 @@ 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 process_map_output (void *cls, const char *line);
+static void
+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,
+              GNUNET_NAT_ERROR_UPNPC_FAILED);
+    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
@@ -311,30 +394,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),
+                  GNUNET_NAT_ERROR_SUCCESS);
         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)
@@ -354,21 +427,23 @@ 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),
+              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));
+              sizeof (mini->current_addr),
+              GNUNET_NAT_ERROR_SUCCESS);
     return;
   }
   /*
    * we're looking for output of the form:
-   * 
+   *
    * "0 TCP  3000->192.168.2.150:3000  'libminiupnpc' ''"
    * "1 UDP  3001->192.168.2.150:3001  'libminiupnpc' ''"
-   * 
+   *
    * the pattern we look for is:
-   * 
+   *
    * "%s TCP  PORT->STRING:OURPORT *" or
    * "%s UDP  PORT->STRING:OURPORT *"
    */
@@ -377,10 +452,10 @@ process_refresh_output (void *cls, const char *line)
     return;                     /* skip */
   if (NULL == strstr (s, pstr))
     return;                     /* skip */
-  if (1 != sscanf (line,
-                   (mini->is_tcp)
-                   ? "%*u TCP  %u->%*s:%*u %*s"
-                   : "%*u UDP  %u->%*s:%*u %*s", &nport))
+  if (1 !=
+      SSCANF (line,
+              (mini->is_tcp) ? "%*u TCP  %u->%*s:%*u %*s" :
+              "%*u UDP  %u->%*s:%*u %*s", &nport))
     return;                     /* skip */
   mini->found = GNUNET_YES;
   if (nport == ntohs (mini->current_addr.sin_port))
@@ -389,16 +464,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),
+            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));
+            sizeof (mini->current_addr),
+            GNUNET_NAT_ERROR_SUCCESS);
 }
 
 
 /**
- * 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
@@ -407,24 +484,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_NO_TASK;
+  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->found = GNUNET_NO;
-  mini->refresh_cmd = GNUNET_OS_command_run (&process_refresh_output,
-                                             mini,
-                                             MAP_TIMEOUT,
-                                             "upnpc", "upnpc", "-l", NULL);
+  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,
+              GNUNET_NAT_ERROR_UPNPC_TIMEOUT);
 }
 
 
 /**
  * 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;
@@ -436,18 +538,24 @@ process_map_output (void *cls, const char *line)
   {
     GNUNET_OS_command_stop (mini->map_cmd);
     mini->map_cmd = NULL;
-    mini->refresh_task = GNUNET_SCHEDULER_add_delayed (MAP_REFRESH_FREQ,
-                                                       &do_refresh, mini);
+    if (GNUNET_YES != mini->did_map)
+      mini->ac (mini->ac_cls,
+                GNUNET_SYSERR,
+                NULL, 0,
+                GNUNET_NAT_ERROR_UPNPC_PORTMAP_FAILED);
+    if (NULL == mini->refresh_task)
+      mini->refresh_task =
+        GNUNET_SCHEDULER_add_delayed (MAP_REFRESH_FREQ, &do_refresh, mini);
     return;
   }
   /*
    * The upnpc output we're after looks like this:
-   * 
+   *
    * "external 87.123.42.204:3000 TCP is redirected to internal 192.168.2.150:3000"
    */
   if ((NULL == (ipaddr = strstr (line, " "))) ||
       (NULL == (pstr = strstr (ipaddr, ":"))) ||
-      (1 != sscanf (pstr + 1, "%u", &port)))
+      (1 != SSCANF (pstr + 1, "%u", &port)))
   {
     return;                     /* skip line */
   }
@@ -468,51 +576,53 @@ 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),
+            GNUNET_NAT_ERROR_SUCCESS);
 }
 
 
 /**
  * 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_MiniAddressCallback ac,
+                           void *ac_cls)
 {
   struct GNUNET_NAT_MiniHandle *ret;
-  char pstr[6];
 
-  if (GNUNET_SYSERR == GNUNET_OS_check_helper_binary ("upnpc"))
+  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,
+        GNUNET_NAT_ERROR_UPNPC_NOT_FOUND);
     return NULL;
-  ret = GNUNET_malloc (sizeof (struct GNUNET_NAT_MiniHandle));
+  }
+  LOG (GNUNET_ERROR_TYPE_DEBUG,
+       "Running `upnpc' to install mapping\n");
+  ret = GNUNET_new (struct GNUNET_NAT_MiniHandle);
   ret->ac = ac;
   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);
-
+  ret->refresh_task =
+    GNUNET_SCHEDULER_add_delayed (MAP_REFRESH_FREQ, &do_refresh, ret);
+  run_upnpc_r (ret);
   return ret;
 }
 
@@ -520,7 +630,7 @@ GNUNET_NAT_mini_map_start (uint16_t port,
 /**
  * Process output from our 'unmap' 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
@@ -530,9 +640,8 @@ process_unmap_output (void *cls, const char *line)
 
   if (NULL == line)
   {
-#if DEBUG_NAT
-    GNUNET_log_from (GNUNET_ERROR_TYPE_DEBUG, "nat", "UPnP unmap done\n");
-#endif
+    LOG (GNUNET_ERROR_TYPE_DEBUG,
+         "UPnP unmap done\n");
     GNUNET_OS_command_stop (mini->unmap_cmd);
     mini->unmap_cmd = NULL;
     GNUNET_free (mini);
@@ -547,7 +656,7 @@ process_unmap_output (void *cls, const char *line)
  * this function will give 'upnpc' 1s to remove tha mapping,
  * so while this function is non-blocking, a task will be
  * left with the scheduler for up to 1s past this call.
- * 
+ *
  * @param mini the handle
  */
 void
@@ -555,47 +664,44 @@ GNUNET_NAT_mini_map_stop (struct GNUNET_NAT_MiniHandle *mini)
 {
   char pstr[6];
 
-  if (GNUNET_SCHEDULER_NO_TASK != mini->refresh_task)
+  if (NULL != mini->refresh_task)
   {
     GNUNET_SCHEDULER_cancel (mini->refresh_task);
-    mini->refresh_task = GNUNET_SCHEDULER_NO_TASK;
+    mini->refresh_task = NULL;
   }
-  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 (NULL != mini->map_cmd)
+  {
+    GNUNET_OS_command_stop (mini->map_cmd);
+    mini->map_cmd = NULL;
+  }
+  if (GNUNET_NO == mini->did_map)
   {
-    if (mini->map_cmd != NULL)
-    {
-      GNUNET_OS_command_stop (mini->map_cmd);
-      mini->map_cmd = NULL;
-    }
     GNUNET_free (mini);
     return;
   }
   mini->ac (mini->ac_cls, GNUNET_NO,
             (const struct sockaddr *) &mini->current_addr,
-            sizeof (mini->current_addr));
+            sizeof (mini->current_addr),
+            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... */
-  GNUNET_snprintf (pstr, sizeof (pstr),
-                   "%u", (unsigned int) ntohs (mini->current_addr.sin_port));
-#if DEBUG_NAT
-  GNUNET_log_from (GNUNET_ERROR_TYPE_DEBUG,
-                   "nat",
-                   "Unmapping port %u with UPnP\n",
-                   ntohs (mini->current_addr.sin_port));
-#endif
-  mini->unmap_cmd = GNUNET_OS_command_run (&process_unmap_output,
-                                           mini,
-                                           UNMAP_TIMEOUT,
-                                           "upnpc",
-                                           "upnpc",
-                                           "-d", pstr,
-                                           mini->is_tcp ? "tcp" : "udp", NULL);
+  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",
+       ntohs (mini->current_addr.sin_port));
+  mini->unmap_cmd =
+      GNUNET_OS_command_run (&process_unmap_output, mini, UNMAP_TIMEOUT,
+                             "upnpc", "upnpc", "-d", pstr,
+                             mini->is_tcp ? "tcp" : "udp", NULL);
 }