again, do not pass salt around, code cleanup, etc.

4 files changed, 259 insertions, 172 deletions

diff --git a/src/set/gnunet-service-set.c b/src/set/gnunet-service-set.c
index 0fc2e4d6a..c3c0d34f6 100644
--- a/src/set/gnunet-service-set.c
+++ b/src/set/gnunet-service-set.c
@@ -26,6 +26,11 @@
 #include "gnunet-service-set.h"
 #include "set_protocol.h"
 
+/**
+ * How long do we hold on to an incoming channel if there is
+ * no local listener before giving up?
+ */
+#define INCOMING_CHANNEL_TIMEOUT GNUNET_TIME_UNIT_MINUTES
 
 /**
  * A listener is inhabited by a client, and waits for evaluation
@@ -195,7 +200,7 @@ listener_destroy (struct Listener *listener)
 
     listener->client = NULL;
     GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                "disconnecting listener client\n");
+                "Disconnecting listener client\n");
     GNUNET_SERVER_client_disconnect (client);
     return;
   }
@@ -475,6 +480,11 @@ incoming_destroy (struct Operation *incoming)
   }
   /* make sure that the tunnel end handler will not destroy us again */
   incoming->vt = NULL;
+  if (NULL != incoming->spec)
+  {
+    GNUNET_free (incoming->spec);
+    incoming->spec = NULL;
+  }
   if (NULL != incoming->mq)
   {
     GNUNET_MQ_destroy (incoming->mq);
@@ -548,7 +558,11 @@ incoming_suggest (struct Operation *incoming,
 
 
 /**
- * Handle a request for a set operation from another peer.
+ * Handle a request for a set operation from another peer.  Checks if we
+ * have a listener waiting for such a request (and in that case initiates
+ * asking the listener about accepting the connection). If no listener
+ * is waiting, we queue the operation request in hope that a listener
+ * shows up soon (before timeout).
  *
  * This msg is expected as the first and only msg handled through the
  * non-operation bound virtual table, acceptance of this operation replaces
@@ -581,11 +595,16 @@ handle_incoming_msg (struct Operation *op,
     GNUNET_break_op (0);
     return GNUNET_SYSERR;
   }
-  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "Got op request\n");
   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
+  /* 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) )
+  {
+    GNUNET_break_op (0);
+    GNUNET_free (spec);
+    return GNUNET_SYSERR;
+  }
   if (NULL != spec->context_msg)
     spec->context_msg = GNUNET_copy_message (spec->context_msg);
   spec->operation = ntohl (msg->operation);
@@ -594,28 +613,23 @@ handle_incoming_msg (struct Operation *op,
                                          UINT32_MAX);
   spec->peer = op->peer;
   spec->remote_element_count = ntohl (msg->element_count);
-
   op->spec = spec;
 
-  if ( (NULL != spec->context_msg) &&
-       (ntohs (spec->context_msg->size) > GNUNET_SET_CONTEXT_MESSAGE_MAX_SIZE) )
-  {
-    GNUNET_break_op (0);
-    return GNUNET_SYSERR;
-  }
-
-  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "received P2P operation request (op %u, app %s)\n",
-              ntohl (msg->operation),
-              GNUNET_h2s (&msg->app_id));
   listener = listener_get_by_target (ntohl (msg->operation),
                                      &msg->app_id);
   if (NULL == listener)
   {
+    GNUNET_break (GNUNET_SCHEDULER_NO_TASK != op->timeout_task);
     GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                "no listener matches incoming request, waiting with timeout\n");
+                "No matching listener for incoming request (op %u, app %s), waiting with timeout\n",
+                ntohl (msg->operation),
+                GNUNET_h2s (&msg->app_id));
     return GNUNET_OK;
   }
+  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+              "Received P2P operation request (op %u, app %s) for active listener\n",
+              ntohl (msg->operation),
+              GNUNET_h2s (&msg->app_id));
   incoming_suggest (op, listener);
   return GNUNET_OK;
 }
@@ -627,6 +641,11 @@ handle_incoming_msg (struct Operation *op,
  * are no more elements in the set.  The caller must ensure that the set's iterator is
  * valid.
  *
+ * The client will acknowledge each received element with a
+ * #GNUNET_MESSAGE_TYPE_SET_ITER_ACK message.  Our
+ * #handle_client_iter_ack() will then trigger the next transmission.
+ * Note that the #GNUNET_MESSAGE_TYPE_SET_ITER_DONE is not acknowledged.
+ *
  * @param set set that should send its next element to its client
  */
 static void
@@ -635,6 +654,7 @@ send_client_element (struct Set *set)
   int ret;
   struct ElementEntry *ee;
   struct GNUNET_MQ_Envelope *ev;
+  struct GNUNET_SET_IterResponseMessage *msg;
 
   GNUNET_assert (NULL != set->iter);
   ret = GNUNET_CONTAINER_multihashmap_iterator_next (set->iter,
@@ -648,13 +668,13 @@ send_client_element (struct Set *set)
   }
   else
   {
-    struct GNUNET_SET_IterResponseMessage *msg;
-
     GNUNET_assert (NULL != ee);
     ev = GNUNET_MQ_msg_extra (msg,
                               ee->element.size,
                               GNUNET_MESSAGE_TYPE_SET_ITER_ELEMENT);
-    memcpy (&msg[1], ee->element.data, ee->element.size);
+    memcpy (&msg[1],
+            ee->element.data,
+            ee->element.size);
     msg->element_type = ee->element.element_type;
   }
   GNUNET_MQ_send (set->client_mq, ev);
@@ -663,6 +683,9 @@ send_client_element (struct Set *set)
 
 /**
  * Called when a client wants to iterate the elements of a set.
+ * Checks if we have a set associated with the client and if we
+ * can right now start an iteration. If all checks out, starts
+ * sending the elements of the set to the client.
  *
  * @param cls unused
  * @param client client that sent the message
@@ -675,33 +698,35 @@ handle_client_iterate (void *cls,
 {
   struct Set *set;
 
-  // iterate over a non existing set
   set = set_get (client);
   if (NULL == set)
   {
+    /* attempt to iterate over a non existing set */
     GNUNET_break (0);
     GNUNET_SERVER_client_disconnect (client);
     return;
   }
-
-  // only one concurrent iterate-action per set
   if (NULL != set->iter)
   {
+    /* Only one concurrent iterate-action allowed per set */
     GNUNET_break (0);
     GNUNET_SERVER_client_disconnect (client);
     return;
   }
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "iterating union set with %u elements\n",
+              "Iterating union set with %u elements\n",
               GNUNET_CONTAINER_multihashmap_size (set->elements));
-  GNUNET_SERVER_receive_done (client, GNUNET_OK);
+  GNUNET_SERVER_receive_done (client,
+                              GNUNET_OK);
   set->iter = GNUNET_CONTAINER_multihashmap_iterator_create (set->elements);
   send_client_element (set);
 }
 
 
 /**
- * Called when a client wants to create a new set.
+ * Called when a client wants to create a new set.  This is typically
+ * the first request from a client, and includes the type of set
+ * operation to be performed.
  *
  * @param cls unused
  * @param client client that sent the message
@@ -717,19 +742,16 @@ handle_client_create_set (void *cls,
 
   msg = (const struct GNUNET_SET_CreateMessage *) m;
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "client created new set (operation %u)\n",
+              "Client created new set (operation %u)\n",
               ntohs (msg->operation));
-
-  // max. one set per client!
   if (NULL != set_get (client))
   {
+    /* There can only be one set per client */
     GNUNET_break (0);
     GNUNET_SERVER_client_disconnect (client);
     return;
   }
-
   set = GNUNET_new (struct Set);
-
   switch (ntohl (msg->operation))
   {
   case GNUNET_SET_OPERATION_INTERSECTION:
@@ -744,13 +766,15 @@ handle_client_create_set (void *cls,
     GNUNET_SERVER_client_disconnect (client);
     return;
   }
-
   set->state = set->vt->create ();
   set->elements = GNUNET_CONTAINER_multihashmap_create (1, GNUNET_YES);
   set->client = client;
   set->client_mq = GNUNET_MQ_queue_for_server_client (client);
-  GNUNET_CONTAINER_DLL_insert (sets_head, sets_tail, set);
-  GNUNET_SERVER_receive_done (client, GNUNET_OK);
+  GNUNET_CONTAINER_DLL_insert (sets_head,
+                               sets_tail,
+                               set);
+  GNUNET_SERVER_receive_done (client,
+                              GNUNET_OK);
 }
 
 
@@ -771,14 +795,13 @@ handle_client_listen (void *cls,
   struct Operation *op;
 
   msg = (const struct GNUNET_SET_ListenMessage *) m;
-  /* max. one per client! */
   if (NULL != listener_get (client))
   {
+    /* max. one active listener per client! */
     GNUNET_break (0);
     GNUNET_SERVER_client_disconnect (client);
     return;
   }
-
   listener = GNUNET_new (struct Listener);
   listener->client = client;
   listener->client_mq = GNUNET_MQ_queue_for_server_client (client);
@@ -788,38 +811,27 @@ handle_client_listen (void *cls,
                                     listeners_tail,
                                     listener);
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "new listener created (op %u, app %s)\n",
+              "New listener created (op %u, app %s)\n",
               listener->operation,
               GNUNET_h2s (&listener->app_id));
 
-  /* check for incoming requests the listener is interested in */
+  /* check for existing incoming requests the listener might be interested in */
   for (op = incoming_head; NULL != op; op = op->next)
   {
     if (NULL == op->spec)
-    {
-      GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                  "request has no spec yet\n");
-      continue;
-    }
-    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                "considering (op: %u, app: %s, suggest: %u)\n",
-                op->spec->operation,
-                GNUNET_h2s (&op->spec->app_id),
-                op->suggest_id);
-
-    /* don't consider the incoming request if it has been already suggested to a listener */
+      continue; /* no details available yet */
     if (0 != op->suggest_id)
-      continue;
+      continue; /* this one has been already suggested to a listener */
     if (listener->operation != op->spec->operation)
-      continue;
-    if (0 != GNUNET_CRYPTO_hash_cmp (&listener->app_id, &op->spec->app_id))
-      continue;
+      continue; /* incompatible operation */
+    if (0 != GNUNET_CRYPTO_hash_cmp (&listener->app_id,
+                                     &op->spec->app_id))
+      continue; /* incompatible appliation */
     GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                "request suggested\n");
-    incoming_suggest (op, listener);
+                "Found matching existing request\n");
+    incoming_suggest (op,
+                      listener);
   }
-  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "considered all incoming requests\n");
   GNUNET_SERVER_receive_done (client, GNUNET_OK);
 }
 
@@ -841,102 +853,143 @@ handle_client_reject (void *cls,
   const struct GNUNET_SET_RejectMessage *msg;
 
   msg = (const struct GNUNET_SET_RejectMessage *) m;
-  GNUNET_break (0 == ntohl (msg->request_id));
-
-  // no matching incoming operation for this reject
   incoming = get_incoming (ntohl (msg->accept_reject_id));
   if (NULL == incoming)
   {
-    GNUNET_SERVER_receive_done (client, GNUNET_SYSERR);
+    /* no matching incoming operation for this reject */
+    GNUNET_break (0);
+    GNUNET_SERVER_receive_done (client,
+                                GNUNET_SYSERR);
     return;
   }
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "peer request rejected by client\n");
-
+              "Peer request (op %u, app %s) rejected by client\n",
+              incoming->spec->operation,
+              GNUNET_h2s (&incoming->spec->app_id));
   GNUNET_CADET_channel_destroy (incoming->channel);
-  //channel destruction handler called immediately upon destruction
-  GNUNET_SERVER_receive_done (client, GNUNET_OK);
+  GNUNET_SERVER_receive_done (client,
+                              GNUNET_OK);
 }
 
 
 /**
- * Called when a client wants to add/remove an element to/from a
- * set it inhabits.
+ * Called when a client wants to add an element to a set it inhabits.
  *
  * @param cls unused
  * @param client client that sent the message
  * @param m message sent by the client
  */
 static void
-handle_client_add_remove (void *cls,
-                          struct GNUNET_SERVER_Client *client,
-                          const struct GNUNET_MessageHeader *m)
+handle_client_add (void *cls,
+                   struct GNUNET_SERVER_Client *client,
+                   const struct GNUNET_MessageHeader *m)
 {
   struct Set *set;
   const struct GNUNET_SET_ElementMessage *msg;
   struct GNUNET_SET_Element el;
   struct ElementEntry *ee;
+  struct ElementEntry *ee_dup;
 
-  // client without a set requested an operation
   set = set_get (client);
   if (NULL == set)
   {
+    /* client without a set requested an operation */
     GNUNET_break (0);
     GNUNET_SERVER_client_disconnect (client);
     return;
   }
-  GNUNET_SERVER_receive_done (client, GNUNET_OK);
+  GNUNET_SERVER_receive_done (client,
+                              GNUNET_OK);
   msg = (const struct GNUNET_SET_ElementMessage *) m;
   el.size = ntohs (m->size) - sizeof *msg;
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "client ins/rem element of size %u\n", el.size);
+              "Client inserts element of size %u\n",
+              el.size);
   el.data = &msg[1];
-  if (GNUNET_MESSAGE_TYPE_SET_REMOVE == ntohs (m->type))
+  ee = GNUNET_malloc (el.size + sizeof *ee);
+  ee->element.size = el.size;
+  memcpy (&ee[1],
+          el.data,
+          el.size);
+  ee->element.data = &ee[1];
+  ee->generation_added = set->current_generation;
+  ee->remote = GNUNET_NO;
+  GNUNET_CRYPTO_hash (ee->element.data,
+                      el.size,
+                      &ee->element_hash);
+  ee_dup = GNUNET_CONTAINER_multihashmap_get (set->elements,
+                                              &ee->element_hash);
+  if (NULL != ee_dup)
   {
-    struct GNUNET_HashCode hash;
+    /* same element inserted twice */
+    GNUNET_break (0);
+    GNUNET_free (ee);
+    return;
+  }
+  GNUNET_break (GNUNET_YES ==
+                GNUNET_CONTAINER_multihashmap_put (set->elements,
+                                                   &ee->element_hash,
+                                                   ee,
+                                                   GNUNET_CONTAINER_MULTIHASHMAPOPTION_UNIQUE_ONLY));
+  set->vt->add (set->state, ee);
+}
 
-    GNUNET_CRYPTO_hash (el.data, el.size, &hash);
-    ee = GNUNET_CONTAINER_multihashmap_get (set->elements, &hash);
-    if (NULL == ee)
-    {
-      GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
-                  "client tried to remove non-existing element\n");
-      return;
-    }
-    if (GNUNET_YES == ee->removed)
-    {
-      GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
-                  "client tried to remove element twice\n");
-      return;
-    }
-    ee->removed = GNUNET_YES;
-    ee->generation_removed = set->current_generation;
-    set->vt->remove (set->state, ee);
+
+/**
+ * Called when a client wants to remove an element from a set it
+ * inhabits.
+ *
+ * @param cls unused
+ * @param client client that sent the message
+ * @param m message sent by the client
+ */
+static void
+handle_client_remove (void *cls,
+                      struct GNUNET_SERVER_Client *client,
+                      const struct GNUNET_MessageHeader *m)
+{
+  struct Set *set;
+  const struct GNUNET_SET_ElementMessage *msg;
+  struct GNUNET_SET_Element el;
+  struct ElementEntry *ee;
+  struct GNUNET_HashCode hash;
+
+  set = set_get (client);
+  if (NULL == set)
+  {
+    /* client without a set requested an operation */
+    GNUNET_break (0);
+    GNUNET_SERVER_client_disconnect (client);
+    return;
   }
-  else
+  GNUNET_SERVER_receive_done (client,
+                              GNUNET_OK);
+  msg = (const struct GNUNET_SET_ElementMessage *) m;
+  el.size = ntohs (m->size) - sizeof *msg;
+  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+              "Client removes element of size %u\n",
+              el.size);
+  el.data = &msg[1];
+  GNUNET_CRYPTO_hash (el.data,
+                      el.size,
+                      &hash);
+  ee = GNUNET_CONTAINER_multihashmap_get (set->elements,
+                                          &hash);
+  if (NULL == ee)
   {
-    struct ElementEntry *ee_dup;
-
-    ee = GNUNET_malloc (el.size + sizeof *ee);
-    ee->element.size = el.size;
-    memcpy (&ee[1], el.data, el.size);
-    ee->element.data = &ee[1];
-    ee->generation_added = set->current_generation;
-    ee->remote = GNUNET_NO;
-    GNUNET_CRYPTO_hash (ee->element.data, el.size, &ee->element_hash);
-    ee_dup = GNUNET_CONTAINER_multihashmap_get (set->elements,
-                                                &ee->element_hash);
-    if (NULL != ee_dup)
-    {
-      GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                  "element inserted twice, ignoring\n");
-      GNUNET_free (ee);
-      return;
-    }
-    GNUNET_CONTAINER_multihashmap_put (set->elements, &ee->element_hash, ee,
-                                       GNUNET_CONTAINER_MULTIHASHMAPOPTION_UNIQUE_ONLY);
-    set->vt->add (set->state, ee);
+    /* Client tried to remove non-existing element */
+    GNUNET_break (0);
+    return;
   }
+  if (GNUNET_YES == ee->removed)
+  {
+    /* Client tried to remove element twice */
+    GNUNET_break (0);
+    return;
+  }
+  ee->removed = GNUNET_YES;
+  ee->generation_removed = set->current_generation;
+  set->vt->remove (set->state, ee);
 }
 
 
@@ -956,6 +1009,7 @@ handle_client_evaluate (void *cls,
   const struct GNUNET_SET_EvaluateMessage *msg;
   struct OperationSpecification *spec;
   struct Operation *op;
+  const struct GNUNET_MessageHeader *context;
 
   set = set_get (client);
   if (NULL == set)
@@ -969,36 +1023,44 @@ handle_client_evaluate (void *cls,
   spec = GNUNET_new (struct OperationSpecification);
   spec->operation = set->operation;
   spec->app_id = msg->app_id;
-  spec->salt = ntohl (msg->salt);
+  spec->salt = GNUNET_CRYPTO_random_u32 (GNUNET_CRYPTO_QUALITY_NONCE,
+                                         UINT32_MAX);
   spec->peer = msg->target_peer;
   spec->set = set;
   spec->result_mode = ntohs (msg->result_mode);
   spec->client_request_id = ntohl (msg->request_id);
-  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)
-    spec->context_msg = GNUNET_copy_message (spec->context_msg);
+  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++;
   op->vt = set->vt;
-  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);
+  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);
-  GNUNET_SERVER_receive_done (client, GNUNET_OK);
+  GNUNET_SERVER_receive_done (client,
+                              GNUNET_OK);
 }
 
 
 /**
- * Handle an ack from a client, and send the next element.
+ * Handle an ack from a client, and send the next element. Note
+ * that we only expect acks for set elements, not after the
+ * #GNUNET_MESSAGE_TYPE_SET_ITER_DONE message.
  *
  * @param cls unused
  * @param client the client
@@ -1011,24 +1073,24 @@ handle_client_iter_ack (void *cls,
 {
   struct Set *set;
 
-  // client without a set requested an operation
   set = set_get (client);
   if (NULL == set)
   {
+    /* client without a set acknowledged receiving a value */
     GNUNET_break (0);
     GNUNET_SERVER_client_disconnect (client);
     return;
   }
-
-  // client sent an ack, but we were not expecting one
   if (NULL == set->iter)
   {
+    /* client sent an ack, but we were not expecting one (as
+       set iteration has finished) */
     GNUNET_break (0);
     GNUNET_SERVER_client_disconnect (client);
     return;
   }
-
-  GNUNET_SERVER_receive_done (client, GNUNET_OK);
+  GNUNET_SERVER_receive_done (client,
+                              GNUNET_OK);
   send_client_element (set);
 }
 
@@ -1283,15 +1345,15 @@ channel_new_cb (void *cls,
     return NULL;
   }
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "new incoming channel\n");
+              "New incoming channel\n");
   incoming = GNUNET_new (struct Operation);
   incoming->is_incoming = GNUNET_YES;
   incoming->peer = *initiator;
   incoming->channel = channel;
   incoming->mq = GNUNET_CADET_mq_create (incoming->channel);
   incoming->vt = &incoming_vt;
-  incoming->timeout_task =
-      GNUNET_SCHEDULER_add_delayed (GNUNET_TIME_UNIT_MINUTES,
+  incoming->timeout_task
+    = GNUNET_SCHEDULER_add_delayed (INCOMING_CHANNEL_TIMEOUT,
                                     &incoming_timeout_cb,
                                     incoming);
   GNUNET_CONTAINER_DLL_insert_tail (incoming_head,
@@ -1405,7 +1467,7 @@ run (void *cls, struct GNUNET_SERVER_Handle *server,
     {handle_client_accept, NULL, GNUNET_MESSAGE_TYPE_SET_ACCEPT,
         sizeof (struct GNUNET_SET_AcceptMessage)},
     {handle_client_iter_ack, NULL, GNUNET_MESSAGE_TYPE_SET_ITER_ACK, 0},
-    {handle_client_add_remove, NULL, GNUNET_MESSAGE_TYPE_SET_ADD, 0},
+    {handle_client_add, NULL, GNUNET_MESSAGE_TYPE_SET_ADD, 0},
     {handle_client_create_set, NULL, GNUNET_MESSAGE_TYPE_SET_CREATE,
         sizeof (struct GNUNET_SET_CreateMessage)},
     {handle_client_iterate, NULL, GNUNET_MESSAGE_TYPE_SET_ITER_REQUEST,
@@ -1415,7 +1477,7 @@ run (void *cls, struct GNUNET_SERVER_Handle *server,
         sizeof (struct GNUNET_SET_ListenMessage)},
     {handle_client_reject, NULL, GNUNET_MESSAGE_TYPE_SET_REJECT,
         sizeof (struct GNUNET_SET_RejectMessage)},
-    {handle_client_add_remove, NULL, GNUNET_MESSAGE_TYPE_SET_REMOVE, 0},
+    {handle_client_remove, NULL, GNUNET_MESSAGE_TYPE_SET_REMOVE, 0},
     {handle_client_cancel, NULL, GNUNET_MESSAGE_TYPE_SET_CANCEL,
         sizeof (struct GNUNET_SET_CancelMessage)},
     {NULL, NULL, 0, 0}
diff --git a/src/set/gnunet-service-set_union.c b/src/set/gnunet-service-set_union.c
index c00728015..88b8856b0 100644
--- a/src/set/gnunet-service-set_union.c
+++ b/src/set/gnunet-service-set_union.c
@@ -355,7 +355,8 @@ 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,
+  ev = GNUNET_MQ_msg_nested_mh (msg,
+                                GNUNET_MESSAGE_TYPE_SET_P2P_OPERATION_REQUEST,
                                 op->spec->context_msg);
 
   if (NULL == ev)
@@ -371,10 +372,11 @@ send_operation_request (struct Operation *op)
   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");
+    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");
-
+    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);
diff --git a/src/set/set.h b/src/set/set.h
index be3a7a0c6..f3aedff4f 100644
--- a/src/set/set.h
+++ b/src/set/set.h
@@ -37,6 +37,11 @@
 
 GNUNET_NETWORK_STRUCT_BEGIN
 
+/**
+ * Message sent by the client to the service to ask starting
+ * a new set to perform operations with.  Includes the desired
+ * set operation type.
+ */
 struct GNUNET_SET_CreateMessage
 {
   /**
@@ -51,6 +56,11 @@ struct GNUNET_SET_CreateMessage
 };
 
 
+/**
+ * Message sent by the client to the service to start listening for
+ * incoming requests to perform a certain type of set operation for a
+ * certain type of application.
+ */
 struct GNUNET_SET_ListenMessage
 {
   /**
@@ -71,6 +81,10 @@ struct GNUNET_SET_ListenMessage
 };
 
 
+/**
+ * Message sent by a listening client to the service to accept
+ * performing the operation with the other peer.
+ */
 struct GNUNET_SET_AcceptMessage
 {
   /**
@@ -79,13 +93,12 @@ struct GNUNET_SET_AcceptMessage
   struct GNUNET_MessageHeader header;
 
   /**
-   * ID of the incoming request we want to accept / reject.
+   * ID of the incoming request we want to accept.
    */
   uint32_t accept_reject_id GNUNET_PACKED;
 
   /**
-   * Request ID to identify responses,
-   * must be 0 if we don't accept the request.
+   * Request ID to identify responses.
    */
   uint32_t request_id GNUNET_PACKED;
 
@@ -97,6 +110,10 @@ struct GNUNET_SET_AcceptMessage
 };
 
 
+/**
+ * Message sent by a listening client to the service to reject
+ * performing the operation with the other peer.
+ */
 struct GNUNET_SET_RejectMessage
 {
   /**
@@ -105,16 +122,10 @@ struct GNUNET_SET_RejectMessage
   struct GNUNET_MessageHeader header;
 
   /**
-   * ID of the incoming request we want to accept / reject.
+   * ID of the incoming request we want to reject.
    */
   uint32_t accept_reject_id GNUNET_PACKED;
 
-  /**
-   * Request ID to identify responses,
-   * must be 0 if we don't accept the request.
-   */
-  uint32_t request_id GNUNET_PACKED;
-
 };
 
 
@@ -143,6 +154,11 @@ struct GNUNET_SET_RequestMessage
 };
 
 
+/**
+ * Message sent by client to service to initiate a set operation as a
+ * client (not as listener).  A set (which determines the operation
+ * type) must already exist in association with this client.
+ */
 struct GNUNET_SET_EvaluateMessage
 {
   /**
@@ -151,9 +167,10 @@ struct GNUNET_SET_EvaluateMessage
   struct GNUNET_MessageHeader header;
 
   /**
-   * id of our evaluate, chosen by the client
+   * How should results be sent to us?
+   * See `enum GNUNET_SET_ResultMode`.
    */
-  uint32_t request_id GNUNET_PACKED;
+  uint32_t result_mode GNUNET_PACKED;
 
   /**
    * Peer to evaluate the operation with
@@ -166,17 +183,13 @@ struct GNUNET_SET_EvaluateMessage
   struct GNUNET_HashCode app_id;
 
   /**
-   * Salt to use for the operation.
-   */
-  uint32_t salt GNUNET_PACKED;
-
-  /**
-   * How should results be sent to us?
-   * See `enum GNUNET_SET_ResultMode`.
+   * Id of our set to evaluate, chosen implicitly by the client when it
+   * calls #GNUNE_SET_commit().
    */
-  uint32_t result_mode GNUNET_PACKED;
+  uint32_t request_id GNUNET_PACKED;
 
-  /* rest: inner message */
+  /* rest: context message, that is, application-specific
+     message to convince listener to pick up */
 };
 
 
@@ -207,6 +220,10 @@ struct GNUNET_SET_ResultMessage
 };
 
 
+/**
+ * Message sent by client to the service to add or remove
+ * an element to/from the set.
+ */
 struct GNUNET_SET_ElementMessage
 {
   /**
@@ -215,8 +232,14 @@ struct GNUNET_SET_ElementMessage
    */
   struct GNUNET_MessageHeader header;
 
+  /**
+   * Type of the element to add or remove.
+   */
   uint16_t element_type GNUNET_PACKED;
 
+  /**
+   * For alignment, always zero.
+   */
   uint16_t reserved GNUNET_PACKED;
 
   /* rest: the actual element */
diff --git a/src/set/set_api.c b/src/set/set_api.c
index 1ef0cc50c..ff0c959e4 100644
--- a/src/set/set_api.c
+++ b/src/set/set_api.c
@@ -325,14 +325,14 @@ handle_request (void *cls,
   const struct GNUNET_SET_RequestMessage *msg = (const struct GNUNET_SET_RequestMessage *) mh;
   struct GNUNET_SET_ListenHandle *lh = cls;
   struct GNUNET_SET_Request *req;
-  struct GNUNET_MessageHeader *context_msg;
+  const struct GNUNET_MessageHeader *context_msg;
 
   LOG (GNUNET_ERROR_TYPE_DEBUG,
        "processing operation request\n");
   req = GNUNET_new (struct GNUNET_SET_Request);
   req->accept_id = ntohl (msg->accept_id);
   context_msg = GNUNET_MQ_extract_nested_mh (msg);
-  /* calling GNUNET_SET_accept in the listen cb will set req->accepted */
+  /* calling #GNUNET_SET_accept() in the listen cb will set req->accepted */
   lh->listen_cb (lh->listen_cls, &msg->peer_id, context_msg, req);
 
   /* we got another request => reset the backoff */
@@ -345,8 +345,6 @@ handle_request (void *cls,
 
     mqm = GNUNET_MQ_msg (rmsg,
                          GNUNET_MESSAGE_TYPE_SET_REJECT);
-    /* no request id, as we refused */
-    rmsg->request_id = htonl (0);
     rmsg->accept_reject_id = msg->accept_id;
     GNUNET_MQ_send (lh->mq, mqm);
     LOG (GNUNET_ERROR_TYPE_DEBUG,
@@ -363,7 +361,8 @@ handle_request (void *cls,
 
 
 static void
-handle_client_listener_error (void *cls, enum GNUNET_MQ_Error error)
+handle_client_listener_error (void *cls,
+                              enum GNUNET_MQ_Error error)
 {
   struct GNUNET_SET_ListenHandle *lh = cls;
 
@@ -639,7 +638,6 @@ GNUNET_SET_prepare (const struct GNUNET_PeerIdentity *other_peer,
   oh = GNUNET_new (struct GNUNET_SET_OperationHandle);
   oh->result_cb = result_cb;
   oh->result_cls = result_cls;
-
   mqm = GNUNET_MQ_msg_nested_mh (msg,
                                  GNUNET_MESSAGE_TYPE_SET_EVALUATE,
                                  context_msg);
@@ -825,7 +823,9 @@ GNUNET_SET_commit (struct GNUNET_SET_OperationHandle *oh,
     return GNUNET_SYSERR;
   GNUNET_assert (NULL != oh->conclude_mqm);
   oh->set = set;
-  GNUNET_CONTAINER_DLL_insert (set->ops_head, set->ops_tail, oh);
+  GNUNET_CONTAINER_DLL_insert (set->ops_head,
+                               set->ops_tail,
+                               oh);
   oh->request_id = GNUNET_MQ_assoc_add (set->mq, oh);
   *oh->request_id_addr = htonl (oh->request_id);
   GNUNET_MQ_send (set->mq, oh->conclude_mqm);