clean up internal set API, avoid copying context message needlessly

6 files changed, 125 insertions, 141 deletions

diff --git a/src/set/gnunet-service-set.c b/src/set/gnunet-service-set.c
index c3c0d34f6..97ee1480d 100644
--- a/src/set/gnunet-service-set.c
+++ b/src/set/gnunet-service-set.c
@@ -549,7 +549,7 @@ incoming_suggest (struct Operation *incoming,
                                  incoming->spec->context_msg);
   GNUNET_assert (NULL != mqm);
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "suggesting request with accept id %u\n",
+              "Suggesting incoming request with accept id %u to listener\n",
               incoming->suggest_id);
   cmsg->accept_id = htonl (incoming->suggest_id);
   cmsg->peer_id = incoming->spec->peer;
@@ -581,6 +581,7 @@ handle_incoming_msg (struct Operation *op,
   const struct OperationRequestMessage *msg;
   struct Listener *listener;
   struct OperationSpecification *spec;
+  const struct GNUNET_MessageHeader *nested_context;
 
   msg = (const struct OperationRequestMessage *) mh;
   GNUNET_assert (GNUNET_YES == op->is_incoming);
@@ -596,17 +597,19 @@ handle_incoming_msg (struct Operation *op,
     return GNUNET_SYSERR;
   }
   spec = GNUNET_new (struct OperationSpecification);
-  spec->context_msg = GNUNET_MQ_extract_nested_mh (msg);
-  /* for simplicity we just backup the context msg instead of rebuilding it later on */
-  if ( (NULL != spec->context_msg) &&
-       (ntohs (spec->context_msg->size) > GNUNET_SET_CONTEXT_MESSAGE_MAX_SIZE) )
+  nested_context = GNUNET_MQ_extract_nested_mh (msg);
+  if ( (NULL != nested_context) &&
+       (ntohs (nested_context->size) > GNUNET_SET_CONTEXT_MESSAGE_MAX_SIZE) )
   {
     GNUNET_break_op (0);
     GNUNET_free (spec);
     return GNUNET_SYSERR;
   }
+  /* Make a copy of the nested_context (application-specific context
+     information that is opaque to set) so we can pass it to the
+     listener later on */
   if (NULL != spec->context_msg)
-    spec->context_msg = GNUNET_copy_message (spec->context_msg);
+    spec->context_msg = GNUNET_copy_message (nested_context);
   spec->operation = ntohl (msg->operation);
   spec->app_id = msg->app_id;
   spec->salt = GNUNET_CRYPTO_random_u32 (GNUNET_CRYPTO_QUALITY_NONCE,
@@ -994,7 +997,9 @@ handle_client_remove (void *cls,
 
 
 /**
- * Called when a client wants to evaluate a set operation with another peer.
+ * Called when a client wants to evaluate a set operation with another
+ * peer.  Initiates the CADET connection to the listener and sends the
+ * request.
  *
  * @param cls unused
  * @param client client that sent the message
@@ -1018,7 +1023,6 @@ handle_client_evaluate (void *cls,
     GNUNET_SERVER_client_disconnect (client);
     return;
   }
-
   msg = (const struct GNUNET_SET_EvaluateMessage *) m;
   spec = GNUNET_new (struct OperationSpecification);
   spec->operation = set->operation;
@@ -1030,13 +1034,6 @@ handle_client_evaluate (void *cls,
   spec->result_mode = ntohs (msg->result_mode);
   spec->client_request_id = ntohl (msg->request_id);
   context = GNUNET_MQ_extract_nested_mh (msg);
-  /* The evaluate message MAY contain a nested message to be passed to
-     the listner for additional authentication or application-specific
-     context.  We make a copy of this nested/context msg as we need
-     to transmit it later. */
-  if (NULL != context)
-    spec->context_msg = GNUNET_copy_message (context);
-
   op = GNUNET_new (struct Operation);
   op->spec = spec;
   op->generation_created = set->current_generation++;
@@ -1044,14 +1041,14 @@ handle_client_evaluate (void *cls,
   GNUNET_CONTAINER_DLL_insert (set->ops_head,
                                set->ops_tail,
                                op);
-
   op->channel = GNUNET_CADET_channel_create (cadet,
                                              op,
                                              &msg->target_peer,
                                              GNUNET_APPLICATION_TYPE_SET,
                                              GNUNET_CADET_OPTION_RELIABLE);
   op->mq = GNUNET_CADET_mq_create (op->channel);
-  set->vt->evaluate (op);
+  set->vt->evaluate (op,
+                     context);
   GNUNET_SERVER_receive_done (client,
                               GNUNET_OK);
 }
@@ -1226,7 +1223,6 @@ handle_client_accept (void *cls,
   op->spec->result_mode = ntohl (msg->result_mode);
   op->generation_created = set->current_generation++;
   op->vt = op->spec->set->vt;
-  GNUNET_assert (NULL != op->vt->accept);
   set->vt->accept (op);
   GNUNET_SERVER_receive_done (client, GNUNET_OK);
 }
diff --git a/src/set/gnunet-service-set.h b/src/set/gnunet-service-set.h
index 834f78780..448ed9478 100644
--- a/src/set/gnunet-service-set.h
+++ b/src/set/gnunet-service-set.h
@@ -165,13 +165,27 @@ typedef void
 
 
 /**
- * Signature of functions that implement the creation of set operations
- * (currently "evaluate" and "accept").
+ * Signature of functions that implement accepting a set operation.
  *
- * @param op operation that is created, should be initialized by the implementation
+ * @param op operation that is created by accepting the operation,
+ *        should be initialized by the implementation
  */
 typedef void
-(*OpCreateImpl) (struct Operation *op);
+(*OpAcceptImpl) (struct Operation *op);
+
+
+/**
+ * Signature of functions that implement starting the evaluation of
+ * set operations.
+ *
+ * @param op operation that is created, should be initialized to
+ *        begin the evaluation
+ * @param opaque_context message to be transmitted to the listener
+ *        to convince him to accept, may be NULL
+ */
+typedef void
+(*OpEvaluateImpl) (struct Operation *op,
+                   const struct GNUNET_MessageHeader *opaque_context);
 
 
 /**
@@ -221,12 +235,12 @@ struct SetVT
   /**
    * Callback for accepting a set operation request
    */
-  OpCreateImpl accept;
+  OpAcceptImpl accept;
 
   /**
    * Callback for starting evaluation with a remote peer.
    */
-  OpCreateImpl evaluate;
+  OpEvaluateImpl evaluate;
 
   /**
    * Callback for destruction of the set state.
diff --git a/src/set/gnunet-service-set_intersection.c b/src/set/gnunet-service-set_intersection.c
index 70c3fb450..734783137 100644
--- a/src/set/gnunet-service-set_intersection.c
+++ b/src/set/gnunet-service-set_intersection.c
@@ -362,49 +362,6 @@ fail_intersection_operation (struct Operation *op)
 }
 
 
-/**
- * Send a request for the evaluate operation to a remote peer
- *
- * @param op operation with the other peer
- */
-static void
-send_operation_request (struct Operation *op)
-{
-  struct GNUNET_MQ_Envelope *ev;
-  struct OperationRequestMessage *msg;
-
-  ev = GNUNET_MQ_msg_nested_mh (msg, GNUNET_MESSAGE_TYPE_SET_P2P_OPERATION_REQUEST,
-                                op->spec->context_msg);
-
-  if (NULL == ev)
-  {
-    /* the context message is too large */
-    GNUNET_break (0);
-    GNUNET_SERVER_client_disconnect (op->spec->set->client);
-    return;
-  }
-  msg->operation = htonl (GNUNET_SET_OPERATION_INTERSECTION);
-  msg->app_id = op->spec->app_id;
-  msg->salt = htonl (op->spec->salt);
-  msg->element_count = htonl(op->state->my_element_count);
-
-  GNUNET_MQ_send (op->mq, ev);
-
-  if (NULL != op->spec->context_msg)
-    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                "sent op request with context message\n");
-  else
-    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                "sent op request without context message\n");
-
-  if (NULL != op->spec->context_msg)
-  {
-    GNUNET_free (op->spec->context_msg);
-    op->spec->context_msg = NULL;
-  }
-}
-
-
 static void
 send_bloomfilter_multipart (struct Operation *op,
                             uint32_t offset)
@@ -877,14 +834,20 @@ handle_p2p_done (void *cls,
 
 
 /**
- * Evaluate a union operation with
- * a remote peer.
+ * Initiate a set union operation with a remote peer.
  *
- * @param op operation to evaluate
+ * @param op operation that is created, should be initialized to
+ *        begin the evaluation
+ * @param opaque_context message to be transmitted to the listener
+ *        to convince him to accept, may be NULL
  */
 static void
-intersection_evaluate (struct Operation *op)
+intersection_evaluate (struct Operation *op,
+                       const struct GNUNET_MessageHeader *opaque_context)
 {
+  struct GNUNET_MQ_Envelope *ev;
+  struct OperationRequestMessage *msg;
+
   op->state = GNUNET_new (struct OperationState);
   /* we started the operation, thus we have to send the operation request */
   op->state->phase = PHASE_INITIAL;
@@ -892,8 +855,28 @@ intersection_evaluate (struct Operation *op)
   op->state->my_element_count = op->spec->set->state->current_set_element_count;
 
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "evaluating intersection operation");
-  send_operation_request (op);
+              "Initiating intersection operation evaluation");
+  ev = GNUNET_MQ_msg_nested_mh (msg,
+                                GNUNET_MESSAGE_TYPE_SET_P2P_OPERATION_REQUEST,
+                                opaque_context);
+  if (NULL == ev)
+  {
+    /* the context message is too large */
+    GNUNET_break (0);
+    GNUNET_SERVER_client_disconnect (op->spec->set->client);
+    return;
+  }
+  msg->operation = htonl (GNUNET_SET_OPERATION_INTERSECTION);
+  msg->app_id = op->spec->app_id;
+  msg->salt = htonl (op->spec->salt);
+  msg->element_count = htonl(op->state->my_element_count);
+  GNUNET_MQ_send (op->mq, ev);
+  if (NULL != opaque_context)
+    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+                "sent op request with context message\n");
+  else
+    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+                "sent op request without context message\n");
 }
 
 
diff --git a/src/set/gnunet-service-set_union.c b/src/set/gnunet-service-set_union.c
index 88b8856b0..5926b25f5 100644
--- a/src/set/gnunet-service-set_union.c
+++ b/src/set/gnunet-service-set_union.c
@@ -345,47 +345,6 @@ get_ibf_key (const struct GNUNET_HashCode *src, uint16_t salt)
 
 
 /**
- * Send a request for the evaluate operation to a remote peer
- *
- * @param op operation with the other peer
- */
-static void
-send_operation_request (struct Operation *op)
-{
-  struct GNUNET_MQ_Envelope *ev;
-  struct OperationRequestMessage *msg;
-
-  ev = GNUNET_MQ_msg_nested_mh (msg,
-                                GNUNET_MESSAGE_TYPE_SET_P2P_OPERATION_REQUEST,
-                                op->spec->context_msg);
-
-  if (NULL == ev)
-  {
-    /* the context message is too large */
-    GNUNET_break (0);
-    GNUNET_SERVER_client_disconnect (op->spec->set->client);
-    return;
-  }
-  msg->operation = htonl (GNUNET_SET_OPERATION_UNION);
-  msg->app_id = op->spec->app_id;
-  msg->salt = htonl (op->spec->salt);
-  GNUNET_MQ_send (op->mq, ev);
-
-  if (NULL != op->spec->context_msg)
-    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                "sent op request with context message\n");
-  else
-    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                "sent op request without context message\n");
-  if (NULL != op->spec->context_msg)
-  {
-    GNUNET_free (op->spec->context_msg);
-    op->spec->context_msg = NULL;
-  }
-}
-
-
-/**
  * Iterator to create the mapping between ibf keys
  * and element entries.
  *
@@ -1209,21 +1168,47 @@ handle_p2p_done (void *cls, const struct GNUNET_MessageHeader *mh)
 
 
 /**
- * Evaluate a union operation with
- * a remote peer.
+ * Initiate operation to evaluate a set union with a remote peer.
  *
- * @param op operation to evaluate
+ * @param op operation to perform (to be initialized)
+ * @param opaque_context message to be transmitted to the listener
+ *        to convince him to accept, may be NULL
  */
 static void
-union_evaluate (struct Operation *op)
+union_evaluate (struct Operation *op,
+                const struct GNUNET_MessageHeader *opaque_context)
 {
+  struct GNUNET_MQ_Envelope *ev;
+  struct OperationRequestMessage *msg;
+
   op->state = GNUNET_new (struct OperationState);
-  // copy the current generation's strata estimator for this operation
+  /* copy the current generation's strata estimator for this operation */
   op->state->se = strata_estimator_dup (op->spec->set->state->se);
   /* we started the operation, thus we have to send the operation request */
   op->state->phase = PHASE_EXPECT_SE;
-  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "evaluating union operation\n");
-  send_operation_request (op);
+  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+              "Initiating union operation evaluation\n");
+  ev = GNUNET_MQ_msg_nested_mh (msg,
+                                GNUNET_MESSAGE_TYPE_SET_P2P_OPERATION_REQUEST,
+                                opaque_context);
+  if (NULL == ev)
+  {
+    /* the context message is too large */
+    GNUNET_break (0);
+    GNUNET_SERVER_client_disconnect (op->spec->set->client);
+    return;
+  }
+  msg->operation = htonl (GNUNET_SET_OPERATION_UNION);
+  msg->app_id = op->spec->app_id;
+  msg->salt = htonl (op->spec->salt);
+  GNUNET_MQ_send (op->mq, ev);
+
+  if (NULL != opaque_context)
+    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+                "sent op request with context message\n");
+  else
+    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+                "sent op request without context message\n");
 }
 
 
diff --git a/src/set/set.h b/src/set/set.h
index f3aedff4f..b470ebf92 100644
--- a/src/set/set.h
+++ b/src/set/set.h
@@ -150,7 +150,8 @@ struct GNUNET_SET_RequestMessage
    */
   struct GNUNET_PeerIdentity peer_id;
 
-  /* rest: nested context message */
+  /* rest: context message, that is, application-specific
+     message to convince listener to pick up */
 };
 
 
diff --git a/src/set/set_api.c b/src/set/set_api.c
index ff0c959e4..90cba446c 100644
--- a/src/set/set_api.c
+++ b/src/set/set_api.c
@@ -402,18 +402,16 @@ set_destroy (struct GNUNET_SET_Handle *set)
 }
 
 
-
-
 /**
  * Cancel the given set operation.  We need to send an explicit cancel message,
  * as all operations one one set communicate using one handle.
  *
- * In contrast to GNUNET_SET_operation_cancel, this function indicates whether
+ * In contrast to #GNUNET_SET_operation_cancel(), this function indicates whether
  * the set of the operation has been destroyed because all operations are done and
  * the set's destruction was requested before.
  *
  * @param oh set operation to cancel
- * @return GNUNET_YES if the set of the operation was destroyed
+ * @return #GNUNET_YES if the set of the operation was destroyed
  */
 static int
 set_operation_cancel (struct GNUNET_SET_OperationHandle *oh)
@@ -430,8 +428,11 @@ set_operation_cancel (struct GNUNET_SET_OperationHandle *oh)
     struct GNUNET_SET_CancelMessage *m;
     struct GNUNET_MQ_Envelope *mqm;
 
-    GNUNET_CONTAINER_DLL_remove (oh->set->ops_head, oh->set->ops_tail, oh);
-    h_assoc = GNUNET_MQ_assoc_remove (oh->set->mq, oh->request_id);
+    GNUNET_CONTAINER_DLL_remove (oh->set->ops_head,
+                                 oh->set->ops_tail,
+                                 oh);
+    h_assoc = GNUNET_MQ_assoc_remove (oh->set->mq,
+                                      oh->request_id);
     GNUNET_assert ((h_assoc == NULL) || (h_assoc == oh));
     mqm = GNUNET_MQ_msg (m, GNUNET_MESSAGE_TYPE_SET_CANCEL);
     m->request_id = htonl (oh->request_id);
@@ -439,20 +440,20 @@ set_operation_cancel (struct GNUNET_SET_OperationHandle *oh)
 
     if (GNUNET_YES == oh->set->destroy_requested)
     {
-      LOG (GNUNET_ERROR_TYPE_DEBUG, "destroying set after operation cancel\n");
+      LOG (GNUNET_ERROR_TYPE_DEBUG,
+           "Destroying set after operation cancel\n");
       ret = set_destroy (oh->set);
     }
   }
-
   GNUNET_free (oh);
-
   return ret;
 }
 
 
 /**
- * Cancel the given set operation.  We need to send an explicit cancel message,
- * as all operations one one set communicate using one handle.
+ * Cancel the given set operation.  We need to send an explicit cancel
+ * message, as all operations one one set communicate using one
+ * handle.
  *
  * @param oh set operation to cancel
  */
@@ -468,7 +469,8 @@ handle_client_set_error (void *cls, enum GNUNET_MQ_Error error)
 {
   struct GNUNET_SET_Handle *set = cls;
 
-  LOG (GNUNET_ERROR_TYPE_DEBUG, "handling client set error\n");
+  LOG (GNUNET_ERROR_TYPE_DEBUG,
+       "handling client set error\n");
 
   while (NULL != set->ops_head)
   {
@@ -588,7 +590,9 @@ GNUNET_SET_remove_element (struct GNUNET_SET_Handle *set,
     return GNUNET_SYSERR;
   }
 
-  mqm = GNUNET_MQ_msg_extra (msg, element->size, GNUNET_MESSAGE_TYPE_SET_REMOVE);
+  mqm = GNUNET_MQ_msg_extra (msg,
+                             element->size,
+                             GNUNET_MESSAGE_TYPE_SET_REMOVE);
   msg->element_type = element->element_type;
   memcpy (&msg[1], element->data, element->size);
   GNUNET_MQ_notify_sent (mqm, cont, cont_cls);
@@ -842,13 +846,15 @@ GNUNET_SET_commit (struct GNUNET_SET_OperationHandle *oh,
  *
  * @param set the set to iterate over
  * @param iter the iterator to call for each element
- * @param cls closure for @a iter
+ * @param iter_cls closure for @a iter
  * @return #GNUNET_YES if the iteration started successfuly,
  *         #GNUNET_NO if another iteration is active
  *         #GNUNET_SYSERR if the set is invalid (e.g. the server crashed, disconnected)
  */
 int
-GNUNET_SET_iterate (struct GNUNET_SET_Handle *set, GNUNET_SET_ElementIterator iter, void *cls)
+GNUNET_SET_iterate (struct GNUNET_SET_Handle *set,
+                    GNUNET_SET_ElementIterator iter,
+                    void *iter_cls)
 {
   struct GNUNET_MQ_Envelope *ev;
 
@@ -859,11 +865,10 @@ GNUNET_SET_iterate (struct GNUNET_SET_Handle *set, GNUNET_SET_ElementIterator it
     return GNUNET_SYSERR;
   if (NULL != set->iterator)
     return GNUNET_NO;
-
-  LOG (GNUNET_ERROR_TYPE_DEBUG, "iterating set\n");
-
+  LOG (GNUNET_ERROR_TYPE_DEBUG,
+       "iterating set\n");
   set->iterator = iter;
-  set->iterator_cls = cls;
+  set->iterator_cls = iter_cls;
   ev = GNUNET_MQ_msg_header (GNUNET_MESSAGE_TYPE_SET_ITER_REQUEST);
   GNUNET_MQ_send (set->mq, ev);
   return GNUNET_YES;