PEERSTORE: Fix hello_add API

The new UPSERT API does not work as indended.
Instead, we now iterate over all entries and check if we have a newer
hello to add/replace.
This also fixes some handle/closure oddities exposed in the peerstore
API.

4 files changed, 197 insertions, 124 deletions

diff --git a/src/include/gnunet_peerstore_service.h b/src/include/gnunet_peerstore_service.h
index 0284d46dd..319946e1d 100644
--- a/src/include/gnunet_peerstore_service.h
+++ b/src/include/gnunet_peerstore_service.h
@@ -181,60 +181,8 @@ typedef void (*GNUNET_PEERSTORE_Continuation) (void *cls, int success);
 /**
  * Context for a add hello uri request.
  */
-struct GNUNET_PEERSTORE_StoreHelloContext
-{
-  /**
-   * Kept (also) in a DLL.
-   */
-  struct GNUNET_PEERSTORE_StoreHelloContext *prev;
-
-  /**
-   * Kept (also) in a DLL.
-   */
-  struct GNUNET_PEERSTORE_StoreHelloContext *next;
-
-  /**
-   * Peerstore handle.
-   */
-  struct GNUNET_PEERSTORE_Handle *h;
-
-  /**
-   * Function to call with information.
-   */
-  GNUNET_PEERSTORE_Continuation cont;
-
-  /**
-   * Closure for @e callback.
-   */
-  void *cont_cls;
-
-  /**
-   * Hello uri which was request for storing.
-   */
-  struct GNUNET_MessageHeader *hello;
-
-  /**
-   * The peer id for the hello.
-   */
-  struct GNUNET_PeerIdentity *pid;
-
-  /**
-   * The iteration for the merge
-   */
-  struct GNUNET_PEERSTORE_StoreContext *sc;
+struct GNUNET_PEERSTORE_StoreHelloContext;
 
-};
-
-/**
- * Closure to hold a GNUNET_PEERSTORE_StoreHelloContext.
- */
-struct GNUNET_PEERSTORE_StoreHelloContextClosure
-{
-  /**
-   * The GNUNET_PEERSTORE_StoreHelloContext to hold.
-   */
-  struct GNUNET_PEERSTORE_StoreHelloContext *shc;
-};
 
 /**
  * Function called by PEERSTORE for each matching record.
diff --git a/src/service/hostlist/gnunet-daemon-hostlist_client.c b/src/service/hostlist/gnunet-daemon-hostlist_client.c
index 9388c8940..95891a1ad 100644
--- a/src/service/hostlist/gnunet-daemon-hostlist_client.c
+++ b/src/service/hostlist/gnunet-daemon-hostlist_client.c
@@ -138,6 +138,26 @@ struct Hostlist
   uint32_t times_used;
 };
 
+/**
+* Context for a add hello uri request.
+*/
+struct StoreHelloEntry
+{
+  /**
+   * Kept (also) in a DLL.
+   */
+  struct StoreHelloEntry *prev;
+
+  /**
+   * Kept (also) in a DLL.
+   */
+  struct StoreHelloEntry *next;
+
+  /**
+   * Store hello ctx
+   */
+  struct GNUNET_PEERSTORE_StoreHelloContext *sc;
+};
 
 /**
  * Our configuration.
@@ -232,12 +252,12 @@ static struct GNUNET_TIME_Absolute end_time;
 /**
  * Head of the linkd list to store the store context for hellos.
  */
-static struct GNUNET_PEERSTORE_StoreHelloContext *shc_head;
+static struct StoreHelloEntry *she_head;
 
 /**
  * Tail of the linkd list to store the store context for hellos.
  */
-static struct GNUNET_PEERSTORE_StoreHelloContext *shc_tail;
+static struct StoreHelloEntry *she_tail;
 
 /**
  * Head of the linked list used to store hostlists
@@ -323,16 +343,17 @@ static struct GNUNET_PEERSTORE_Handle *peerstore;
 static void
 shc_cont (void *cls, int success)
 {
-  struct GNUNET_PEERSTORE_StoreHelloContextClosure *shc_cls =  cls;
+  struct StoreHelloEntry *she =  cls;
 
+  she->sc = NULL;
   if (GNUNET_YES == success)
     GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
                 "Hostlist entry stored successfully!\n");
   else
     GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
                 "Error storing hostlist entry!\n");
-  GNUNET_CONTAINER_DLL_remove (shc_head, shc_tail, shc_cls->shc);
-  GNUNET_free (shc_cls);
+  GNUNET_CONTAINER_DLL_remove (she_head, she_tail, she);
+  GNUNET_free (she);
 }
 
 
@@ -349,8 +370,7 @@ static size_t
 callback_download (void *ptr, size_t size, size_t nmemb, void *ctx)
 {
   static char download_buffer[GNUNET_MAX_MESSAGE_SIZE - 1];
-  struct GNUNET_PEERSTORE_StoreHelloContext *shc;
-  struct GNUNET_PEERSTORE_StoreHelloContextClosure *shc_cls;
+  struct StoreHelloEntry *she;
   const char *cbuf = ptr;
   const struct GNUNET_MessageHeader *msg;
   size_t total;
@@ -414,18 +434,17 @@ callback_download (void *ptr, size_t size, size_t nmemb, void *ctx)
       1,
       GNUNET_NO);
     stat_hellos_obtained++;
-    shc_cls = GNUNET_new (struct GNUNET_PEERSTORE_StoreHelloContextClosure);
-    shc = GNUNET_PEERSTORE_hello_add (peerstore,
-                                      msg,
-                                      shc_cont,
-                                      shc_cls);
-    if (NULL != shc)
+    she = GNUNET_new (struct StoreHelloEntry);
+    she->sc = GNUNET_PEERSTORE_hello_add (peerstore,
+                                           msg,
+                                           shc_cont,
+                                           she);
+    if (NULL != she->sc)
     {
-      shc_cls->shc = shc;
-      GNUNET_CONTAINER_DLL_insert (shc_head, shc_tail, shc);
+      GNUNET_CONTAINER_DLL_insert (she_head, she_tail, she);
     }
     else
-      GNUNET_free (shc_cls);
+      GNUNET_free (she);
     memmove (download_buffer, &download_buffer[msize], download_pos - msize);
     download_pos -= msize;
   }
@@ -1042,28 +1061,40 @@ download_hostlist ()
   CURL_EASY_SETOPT (curl, CURLOPT_FOLLOWLOCATION, 1);
 #ifdef CURLOPT_REDIR_PROTOCOLS_STR
   if (0 == strncasecmp (current_url, "https://", strlen ("https://")))
-    GNUNET_assert (CURLE_OK == curl_easy_setopt (curl, CURLOPT_REDIR_PROTOCOLS_STR, "https"));
+    GNUNET_assert (CURLE_OK == curl_easy_setopt (curl,
+                                                 CURLOPT_REDIR_PROTOCOLS_STR,
+                                                 "https"));
   else
-    GNUNET_assert (CURLE_OK == curl_easy_setopt (curl, CURLOPT_REDIR_PROTOCOLS_STR, "http,https"));
+    GNUNET_assert (CURLE_OK == curl_easy_setopt (curl,
+                                                 CURLOPT_REDIR_PROTOCOLS_STR,
+                                                 "http,https"));
 #else
 #ifdef CURLOPT_REDIR_PROTOCOLS
   if (0 == strncasecmp (current_url, "https://", strlen ("https://")))
-    GNUNET_assert (CURLE_OK == curl_easy_setopt (curl, CURLOPT_REDIR_PROTOCOLS, CURLPROTO_HTTPS));
+    GNUNET_assert (CURLE_OK == curl_easy_setopt (curl, CURLOPT_REDIR_PROTOCOLS,
+                                                 CURLPROTO_HTTPS));
   else
-    GNUNET_assert (CURLE_OK == curl_easy_setopt (curl, CURLOPT_REDIR_PROTOCOLS, CURLPROTO_HTTP | CURLPROTO_HTTPS));
+    GNUNET_assert (CURLE_OK == curl_easy_setopt (curl, CURLOPT_REDIR_PROTOCOLS,
+                                                 CURLPROTO_HTTP
+                                                 | CURLPROTO_HTTPS));
 #endif
 #endif
 #ifdef CURLOPT_PROTOCOLS_STR
   if (0 == strncasecmp (current_url, "https://", strlen ("https://")))
-    GNUNET_assert (CURLE_OK == curl_easy_setopt (curl, CURLOPT_PROTOCOLS_STR, "https"));
+    GNUNET_assert (CURLE_OK == curl_easy_setopt (curl, CURLOPT_PROTOCOLS_STR,
+                                                 "https"));
   else
-    GNUNET_assert (CURLE_OK == curl_easy_setopt (curl, CURLOPT_PROTOCOLS_STR, "http,https"));
+    GNUNET_assert (CURLE_OK == curl_easy_setopt (curl, CURLOPT_PROTOCOLS_STR,
+                                                 "http,https"));
 #else
 #ifdef CURLOPT_PROTOCOLS
   if (0 == strncasecmp (current_url, "https://", strlen ("https://")))
-    GNUNET_assert (CURLE_OK == curl_easy_setopt (curl, CURLOPT_PROTOCOLS, CURLPROTO_HTTPS));
+    GNUNET_assert (CURLE_OK == curl_easy_setopt (curl, CURLOPT_PROTOCOLS,
+                                                 CURLPROTO_HTTPS));
   else
-    GNUNET_assert (CURLE_OK == curl_easy_setopt (curl, CURLOPT_PROTOCOLS, CURLPROTO_HTTP | CURLPROTO_HTTPS));
+    GNUNET_assert (CURLE_OK == curl_easy_setopt (curl, CURLOPT_PROTOCOLS,
+                                                 CURLPROTO_HTTP
+                                                 | CURLPROTO_HTTPS));
 #endif
 #endif
   CURL_EASY_SETOPT (curl, CURLOPT_MAXREDIRS, 4);
@@ -1784,13 +1815,14 @@ GNUNET_HOSTLIST_client_start (const struct GNUNET_CONFIGURATION_Handle *c,
 void
 GNUNET_HOSTLIST_client_stop ()
 {
-  struct GNUNET_PEERSTORE_StoreHelloContext *pos;
+  struct StoreHelloEntry *pos;
 
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Hostlist client shutdown\n");
-  while (NULL != (pos = shc_head))
+  while (NULL != (pos = she_head))
   {
-    GNUNET_CONTAINER_DLL_remove (shc_head, shc_tail, pos);
-    GNUNET_PEERSTORE_hello_add_cancel (pos);
+    GNUNET_CONTAINER_DLL_remove (she_head, she_tail, pos);
+    GNUNET_PEERSTORE_hello_add_cancel (pos->sc);
+    GNUNET_free (pos);
   }
   if (NULL != sget)
   {
diff --git a/src/service/peerstore/peerstore_api.c b/src/service/peerstore/peerstore_api.c
index 6272ebdff..bf8b990e5 100644
--- a/src/service/peerstore/peerstore_api.c
+++ b/src/service/peerstore/peerstore_api.c
@@ -23,6 +23,7 @@
  * @author Omar Tarabai
  * @author Christian Grothoff
  */
+#include "gnunet_time_lib.h"
 #include "platform.h"
 #include "gnunet_common.h"
 #include "gnunet_protocols.h"
@@ -96,6 +97,47 @@ struct GNUNET_PEERSTORE_Handle
 };
 
 /**
+* Context for a add hello uri request.
+*/
+struct GNUNET_PEERSTORE_StoreHelloContext
+{
+  /**
+   * Peerstore handle.
+   */
+  struct GNUNET_PEERSTORE_Handle *h;
+
+  /**
+   * Function to call with information.
+   */
+  GNUNET_PEERSTORE_Continuation cont;
+
+  /**
+   * Closure for @e callback.
+   */
+  void *cont_cls;
+
+  /**
+   * Hello uri which was request for storing.
+   */
+  struct GNUNET_MessageHeader *hello;
+
+  /**
+   * The peer id for the hello.
+   */
+  struct GNUNET_PeerIdentity pid;
+
+  /**
+   * Store operation for the merge
+   */
+  struct GNUNET_PEERSTORE_StoreContext *sc;
+
+  /**
+   * The iteration for the merge
+   */
+  struct GNUNET_PEERSTORE_IterateContext *ic;
+};
+
+/**
  * Context for a store request
  */
 struct GNUNET_PEERSTORE_StoreContext
@@ -635,7 +677,7 @@ handle_iterate_result (void *cls, const struct PeerstoreRecordMessage *msg)
  */
 void
 GNUNET_PEERSTORE_iteration_next (struct GNUNET_PEERSTORE_IterateContext *ic,
-                               uint64_t limit)
+                                 uint64_t limit)
 {
   struct GNUNET_MQ_Envelope *ev;
   struct PeerstoreIterationNextMessage *inm;
@@ -686,11 +728,11 @@ GNUNET_PEERSTORE_iteration_stop (struct GNUNET_PEERSTORE_IterateContext *ic)
 
 struct GNUNET_PEERSTORE_IterateContext *
 GNUNET_PEERSTORE_iteration_start (struct GNUNET_PEERSTORE_Handle *h,
-                                const char *sub_system,
-                                const struct GNUNET_PeerIdentity *peer,
-                                const char *key,
-                                GNUNET_PEERSTORE_Processor callback,
-                                void *callback_cls)
+                                  const char *sub_system,
+                                  const struct GNUNET_PeerIdentity *peer,
+                                  const char *key,
+                                  GNUNET_PEERSTORE_Processor callback,
+                                  void *callback_cls)
 {
   struct GNUNET_MQ_Envelope *ev;
   struct PeerstoreIterationStartMessage *srm;
@@ -806,17 +848,50 @@ hello_store_success (void *cls, int success)
          "Storing hello uri failed\n");
     huc->cont (huc->cont_cls, success);
     GNUNET_free (huc->hello);
-    GNUNET_free (huc->pid);
     GNUNET_free (huc);
     return;
   }
   huc->cont (huc->cont_cls, GNUNET_OK);
   GNUNET_free (huc->hello);
-  GNUNET_free (huc->pid);
   GNUNET_free (huc);
 }
 
 
+static void
+hello_add_iter (void *cls, const struct GNUNET_PEERSTORE_Record *record,
+                const char *emsg)
+{
+  struct GNUNET_PEERSTORE_StoreHelloContext *huc = cls;
+  struct GNUNET_TIME_Absolute hello_exp =
+    GNUNET_HELLO_builder_get_expiration_time (huc->hello);
+  if (NULL == record)
+  {
+    /** If we ever get here, we are newer than the existing record
+     *  or the only/first record.
+     */
+    huc->sc = GNUNET_PEERSTORE_store (huc->h,
+                                      "peerstore",
+                                      &huc->pid,
+                                      GNUNET_PEERSTORE_HELLO_KEY,
+                                      huc->hello,
+                                      ntohs (huc->hello->size),
+                                      hello_exp,
+                                      GNUNET_PEERSTORE_STOREOPTION_UPSERT_LATER_EXPIRY,
+                                      &hello_store_success,
+                                      huc);
+    return;
+  }
+  if (GNUNET_TIME_absolute_cmp (record->expiry, >, hello_exp))
+  {
+    huc->cont (huc->cont_cls, GNUNET_OK);
+    GNUNET_PEERSTORE_iteration_stop (huc->ic);
+    GNUNET_free (huc->hello);
+    GNUNET_free (huc);
+    return;
+  }
+}
+
+
 struct GNUNET_PEERSTORE_StoreHelloContext *
 GNUNET_PEERSTORE_hello_add (struct GNUNET_PEERSTORE_Handle *h,
                             const struct GNUNET_MessageHeader *msg,
@@ -830,7 +905,6 @@ GNUNET_PEERSTORE_hello_add (struct GNUNET_PEERSTORE_Handle *h,
   struct GNUNET_TIME_Absolute hello_exp =
     GNUNET_HELLO_builder_get_expiration_time (msg);
   struct GNUNET_TIME_Absolute huc_exp;
-  uint16_t pid_size;
   uint16_t size_msg = ntohs (msg->size);
 
   if (NULL == builder)
@@ -847,24 +921,17 @@ GNUNET_PEERSTORE_hello_add (struct GNUNET_PEERSTORE_Handle *h,
   huc_exp =
     GNUNET_HELLO_builder_get_expiration_time (huc->hello);
   pid = GNUNET_HELLO_builder_get_id (builder);
-  pid_size = sizeof (struct GNUNET_PeerIdentity);
-  huc->pid = GNUNET_malloc (pid_size);
-  GNUNET_memcpy (huc->pid, pid, pid_size);
+  huc->pid = *pid;
   LOG (GNUNET_ERROR_TYPE_DEBUG,
        "Adding hello for peer %s with expiration %s msg size %u\n",
-       GNUNET_i2s (huc->pid),
+       GNUNET_i2s (&huc->pid),
        GNUNET_STRINGS_absolute_time_to_string (huc_exp),
        size_msg);
-  huc->sc = GNUNET_PEERSTORE_store (h,
-                                    "peerstore",
-                                    huc->pid,
-                                    GNUNET_PEERSTORE_HELLO_KEY,
-                                    huc->hello,
-                                    ntohs (huc->hello->size),
-                                    hello_exp,
-                                    GNUNET_PEERSTORE_STOREOPTION_UPSERT_LATER_EXPIRY,
-                                    &hello_store_success,
-                                    huc);
+
+  huc->ic = GNUNET_PEERSTORE_iteration_start (h, "peerstore", &huc->pid,
+                                              GNUNET_PEERSTORE_HELLO_KEY,
+                                              &hello_add_iter,
+                                              huc);
   GNUNET_HELLO_builder_free (builder);
   return huc;
 }
@@ -874,9 +941,11 @@ void
 GNUNET_PEERSTORE_hello_add_cancel (struct
                                    GNUNET_PEERSTORE_StoreHelloContext *huc)
 {
-  GNUNET_PEERSTORE_store_cancel (huc->sc);
+  if (NULL != huc->sc)
+    GNUNET_PEERSTORE_store_cancel (huc->sc);
+  if (NULL != huc->ic)
+    GNUNET_PEERSTORE_iteration_stop (huc->ic);
   GNUNET_free (huc->hello);
-  GNUNET_free (huc->pid);
   GNUNET_free (huc);
 }
 
diff --git a/src/service/topology/gnunet-daemon-topology.c b/src/service/topology/gnunet-daemon-topology.c
index 588000358..c59610014 100644
--- a/src/service/topology/gnunet-daemon-topology.c
+++ b/src/service/topology/gnunet-daemon-topology.c
@@ -107,6 +107,26 @@ struct Peer
 
 };
 
+/**
+* Context for a add hello uri request.
+*/
+struct StoreHelloEntry
+{
+  /**
+   * Kept (also) in a DLL.
+   */
+  struct StoreHelloEntry *prev;
+
+  /**
+   * Kept (also) in a DLL.
+   */
+  struct StoreHelloEntry *next;
+
+  /**
+   * Store hello ctx
+   */
+  struct GNUNET_PEERSTORE_StoreHelloContext *sc;
+};
 
 /**
  * The task to delayed start the notification process intially.
@@ -182,12 +202,12 @@ static unsigned int target_connection_count;
 /**
  * Head of the linkd list to store the store context for hellos.
  */
-static struct GNUNET_PEERSTORE_StoreHelloContext *shc_head;
+static struct StoreHelloEntry *she_head;
 
 /**
  * Tail of the linkd list to store the store context for hellos.
  */
-static struct GNUNET_PEERSTORE_StoreHelloContext *shc_tail;
+static struct StoreHelloEntry *she_tail;
 
 /**
  * Free all resources associated with the given peer.
@@ -730,10 +750,12 @@ static void
 error_cb (void *cls)
 {
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              _ ("Error in communication with PEERSTORE service to monitor.\n"));
+              _ (
+                "Error in communication with PEERSTORE service to monitor.\n"));
   return;
 }
 
+
 static void
 sync_cb (void *cls)
 {
@@ -742,6 +764,7 @@ sync_cb (void *cls)
   return;
 }
 
+
 /**
  * PEERSTORE calls this function to let us know about a possible peer
  * that we might want to connect to.
@@ -886,16 +909,17 @@ check_hello (void *cls, const struct GNUNET_MessageHeader *message)
 static void
 shc_cont (void *cls, int success)
 {
-  struct GNUNET_PEERSTORE_StoreHelloContextClosure *shc_cls =  cls;
+  struct StoreHelloEntry *she =  cls;
 
+  she->sc = NULL;
   if (GNUNET_YES == success)
     GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
                 "Hello stored successfully!\n");
   else
     GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
                 "Error storing hello!\n");
-  GNUNET_CONTAINER_DLL_remove (shc_head, shc_tail, shc_cls->shc);
-  GNUNET_free (shc_cls);
+  GNUNET_CONTAINER_DLL_remove (she_head, she_tail, she);
+  GNUNET_free (she);
 }
 
 
@@ -909,8 +933,7 @@ shc_cont (void *cls, int success)
 static void
 handle_hello (void *cls, const struct GNUNET_MessageHeader *message)
 {
-  struct GNUNET_PEERSTORE_StoreHelloContext *shc;
-  struct GNUNET_PEERSTORE_StoreHelloContextClosure *shc_cls;
+  struct StoreHelloEntry *she;
   const struct GNUNET_PeerIdentity *other = cls;
   struct GNUNET_HELLO_Builder *builder = GNUNET_HELLO_builder_from_msg (
     message);
@@ -923,15 +946,14 @@ handle_hello (void *cls, const struct GNUNET_MessageHeader *message)
                             1,
                             GNUNET_NO);
   GNUNET_HELLO_builder_from_msg (message);
-  shc_cls = GNUNET_new (struct GNUNET_PEERSTORE_StoreHelloContextClosure);
-  shc = GNUNET_PEERSTORE_hello_add (ps, message, &shc_cont, shc_cls);
-  if (NULL != shc)
+  she = GNUNET_new (struct StoreHelloEntry);
+  she->sc = GNUNET_PEERSTORE_hello_add (ps, message, &shc_cont, she);
+  if (NULL != she->sc)
   {
-    shc_cls->shc = shc;
-    GNUNET_CONTAINER_DLL_insert (shc_head, shc_tail, shc);
+    GNUNET_CONTAINER_DLL_insert (she_head, she_tail, she);
   }
   else
-    GNUNET_free (shc_cls);
+    GNUNET_free (she);
   GNUNET_HELLO_builder_free (builder);
 }
 
@@ -945,13 +967,15 @@ handle_hello (void *cls, const struct GNUNET_MessageHeader *message)
 static void
 cleaning_task (void *cls)
 {
-  struct GNUNET_PEERSTORE_StoreHelloContext *pos;
+  struct StoreHelloEntry *pos;
 
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Topology shutdown\n");
-  while (NULL != (pos = shc_head))
+  while (NULL != (pos = she_head))
   {
-    GNUNET_CONTAINER_DLL_remove (shc_head, shc_tail, pos);
-    GNUNET_PEERSTORE_hello_add_cancel (pos);
+    GNUNET_CONTAINER_DLL_remove (she_head, she_tail, pos);
+    if (NULL != pos->sc)
+      GNUNET_PEERSTORE_hello_add_cancel (pos->sc);
+    GNUNET_free (pos);
   }
   if (NULL != peerstore_notify)
   {