a read/take returning no data must be a no-op

In all cases where read/take allocates memory for storing samples but
the result turns out to be an empty set, the (observable) state of the
system should end up unchanged.

It turns out several cases were/are considered:

* application supplies buffers (i.e., buf[0] != NULL): no memory
  allocated, so no issue.

* reader has no cached set ("m_loan" in the current code): read/take
  allocated memory, cached the address and marked it as in use
  ("m_loan_out"), and modified buf[0] (and subsequent entries).

  To undo this on returning an empty set, it now: resets the
  "m_loan_out" flag to allow the cached buffer to be reused, and sets
  buf[0] back to NULL.

* reader has a cached set, but it is not marked in use: read/take
  marked it as in use and modified buf[0] (and subsequent entries).

  To undo this, it now resets "m_loan_out" to indicate the cached buffer
  is not in use, and sets buf[0] back to NULL.

* reader has a cached set that is currently in use: read/take allocated
  memory and updated buf[0] (and subsequent entries) but left the cached
  state alone.

  To undo this, it now frees the memory and sets buf[0] back to NULL.

With this, in any path where the application lets dds_read/dds_take
allocate memory for the samples:

* it can still safely call dds_return_loan with buf[0] and the actual
  return value of read/take (even if an error code), and whatever memory
  was allocated will not be leaked;

* but it no longer has to do so when the result was empty (or error).

Signed-off-by: Erik Boasson <eb@ilities.com>

src/core/ddsc/src/dds_read.c

@@ -94,6 +94,10 @@ dds_read_impl(
     dds_retcode_t rc;
     struct dds_reader * rd;
     struct dds_readcond * cond;
+    unsigned nodata_cleanups = 0;
+#define NC_CLEAR_LOAN_OUT 1u
+#define NC_FREE_BUF 2u
+#define NC_RESET_BUF 4u
 
     if (buf == NULL) {
         DDS_ERROR("The provided buffer is NULL\n");
@@ -140,14 +144,17 @@ dds_read_impl(
         /* Allocate, use or reallocate loan cached on reader */
         if (rd->m_loan_out) {
             ddsi_sertopic_realloc_samples (buf, rd->m_topic->m_stopic, NULL, 0, maxs);
+            nodata_cleanups = NC_FREE_BUF | NC_RESET_BUF;
         } else {
             if (rd->m_loan) {
                 if (rd->m_loan_size < maxs) {
                     ddsi_sertopic_realloc_samples (buf, rd->m_topic->m_stopic, rd->m_loan, rd->m_loan_size, maxs);
                     rd->m_loan = buf[0];
                     rd->m_loan_size = maxs;
+                    nodata_cleanups = NC_RESET_BUF;
                 } else {
                     buf[0] = rd->m_loan;
+                    nodata_cleanups = NC_RESET_BUF;
                 }
             } else {
                 ddsi_sertopic_realloc_samples (buf, rd->m_topic->m_stopic, NULL, 0, maxs);
@@ -155,6 +162,7 @@ dds_read_impl(
                 rd->m_loan_size = maxs;
             }
             rd->m_loan_out = true;
+            nodata_cleanups |= NC_CLEAR_LOAN_OUT;
         }
     }
 
@@ -173,6 +181,21 @@ dds_read_impl(
     } else {
         ret = (dds_return_t) dds_rhc_read (rd->m_rd->rhc, lock, buf, si, maxs, mask, hand, cond);
     }
+
+    /* if no data read, restore the state to what it was before the call, with the sole
+       exception of holding on to a buffer we just allocated and that is pointed to by
+       rd->m_loan */
+    if (ret <= 0 && nodata_cleanups) {
+        if (nodata_cleanups & NC_CLEAR_LOAN_OUT) {
+            rd->m_loan_out = false;
+        }
+        if (nodata_cleanups & NC_FREE_BUF) {
+            ddsi_sertopic_free_samples (rd->m_topic->m_stopic, buf[0], maxs, DDS_FREE_ALL);
+        }
+        if (nodata_cleanups & NC_RESET_BUF) {
+            buf[0] = NULL;
+        }
+    }
     dds_read_unlock(rd, cond);
 
 fail_awake: