Fix confusion between raw samples and CDR in PSMX

src/core/ddsc/src/dds__write.h

@@ -32,13 +32,16 @@ typedef enum {
 
 /** @component write_data */
 DDS_EXPORT_INTERNAL_FUNCTION
-dds_return_t dds_write_impl (dds_writer *wr, const void *data, dds_time_t tstamp, dds_write_action action);
+dds_return_t dds_write_impl (dds_writer *wr, const void *data, dds_time_t timestamp, dds_write_action action)
+  ddsrt_attribute_warn_unused_result ddsrt_nonnull_all;
 
 /** @component write_data */
-dds_return_t dds_writecdr_impl (dds_writer *wr, struct ddsi_xpack *xp, struct ddsi_serdata *d, bool flush);
+dds_return_t dds_writecdr_impl (dds_writer *wr, struct ddsi_xpack *xp, struct ddsi_serdata *d, bool flush)
+  ddsrt_attribute_warn_unused_result ddsrt_nonnull_all;
 
 /** @component write_data */
-dds_return_t dds_writecdr_local_orphan_impl (struct ddsi_local_orphan_writer *lowr, struct ddsi_serdata *d);
+dds_return_t dds_writecdr_local_orphan_impl (struct ddsi_local_orphan_writer *lowr, struct ddsi_serdata *d)
+  ddsrt_nonnull_all;
 
 /** @component write_data */
 void dds_write_flush_impl (dds_writer *wr);

src/core/ddsc/src/dds_heap_loan.c

@@ -47,7 +47,7 @@ const dds_loaned_sample_ops_t dds_loan_heap_ops = {
 
 dds_return_t dds_heap_loan (const struct ddsi_sertype *type, dds_loaned_sample_state_t sample_state, struct dds_loaned_sample **loaned_sample)
 {
-  assert (sample_state == DDS_LOANED_SAMPLE_STATE_RAW_KEY || sample_state == DDS_LOANED_SAMPLE_STATE_RAW_DATA);
+  assert (sample_state == DDS_LOANED_SAMPLE_STATE_UNITIALIZED || sample_state == DDS_LOANED_SAMPLE_STATE_RAW_KEY || sample_state == DDS_LOANED_SAMPLE_STATE_RAW_DATA);
 
   dds_heap_loan_t *s = ddsrt_malloc (sizeof (*s));
   if (s == NULL)
@@ -65,6 +65,9 @@ dds_return_t dds_heap_loan (const struct ddsi_sertype *type, dds_loaned_sample_s
   s->c.metadata->sample_state = sample_state;
   s->c.metadata->cdr_identifier = DDSI_RTPS_SAMPLE_NATIVE;
   s->c.metadata->cdr_options = 0;
+  s->c.metadata->sample_size = type->sizeof_type;
+  s->c.metadata->instance_id = 0;
+  s->c.metadata->data_type = 0;
   s->c.loan_origin.origin_kind = DDS_LOAN_ORIGIN_KIND_HEAP;
   s->c.loan_origin.psmx_endpoint = NULL;
   ddsrt_atomic_st32 (&s->c.refc, 1);

src/core/ddsc/src/dds_serdata_default.c

@@ -513,9 +513,10 @@ static struct ddsi_serdata *serdata_default_from_keyhash_cdr_nokey (const struct
 
 static void istream_from_serdata_default (dds_istream_t * __restrict s, const struct dds_serdata_default * __restrict d)
 {
-  if (d->c.loan != NULL)
+  if (d->c.loan != NULL &&
+      (d->c.loan->metadata->sample_state == DDS_LOANED_SAMPLE_STATE_SERIALIZED_KEY ||
+       d->c.loan->metadata->sample_state == DDS_LOANED_SAMPLE_STATE_SERIALIZED_DATA))
   {
-    assert (d->c.loan->metadata->sample_state == DDS_LOANED_SAMPLE_STATE_SERIALIZED_KEY || d->c.loan->metadata->sample_state == DDS_LOANED_SAMPLE_STATE_SERIALIZED_DATA);
     s->m_buffer = d->c.loan->sample_ptr;
     s->m_index = 0;
     s->m_size = d->c.loan->metadata->sample_size;
@@ -715,6 +716,7 @@ static bool serdata_default_to_sample_cdr (const struct ddsi_serdata *serdata_co
   dds_istream_t is;
   if (bufptr) abort(); else { (void)buflim; } /* FIXME: haven't implemented that bit yet! */
   if (d->c.loan != NULL &&
+      tp->c.is_memcpy_safe &&
       (d->c.loan->metadata->sample_state == DDS_LOANED_SAMPLE_STATE_RAW_DATA ||
        d->c.loan->metadata->sample_state == DDS_LOANED_SAMPLE_STATE_RAW_KEY))
   {
@@ -844,68 +846,50 @@ static bool loaned_sample_state_to_serdata_kind (dds_loaned_sample_state_t lss,
   return false;
 }
 
-static struct ddsi_serdata *serdata_default_from_loaned_sample (const struct ddsi_sertype *type, enum ddsi_serdata_kind kind, const char *sample, dds_loaned_sample_t *loaned_sample, bool force_serialization)
+static struct ddsi_serdata *serdata_default_from_loaned_sample (const struct ddsi_sertype *type, enum ddsi_serdata_kind kind, const char *sample, dds_loaned_sample_t *loaned_sample, bool will_require_cdr)
 {
   /*
     type = the type of data being serialized
     kind = the kind of data contained (key or normal)
     sample = the raw sample made into the serdata
     loaned_sample = the loaned buffer in use
-    force_serialization = whether the contents of the loaned sample should be serialized
+    will_require_cdr = whether we will need the CDR (or a highly likely to need it)
   */
   const struct dds_sertype_default *tp = (const struct dds_sertype_default *) type;
 
-  assert (loaned_sample->loan_origin.origin_kind == DDS_LOAN_ORIGIN_KIND_PSMX);
-  bool serialize_data = force_serialization || !type->is_memcpy_safe;
+  assert (sample == loaned_sample->sample_ptr);
+  assert (loaned_sample->metadata->sample_state == (kind == SDK_KEY ? DDS_LOANED_SAMPLE_STATE_RAW_KEY : DDS_LOANED_SAMPLE_STATE_RAW_DATA));
+  assert (loaned_sample->metadata->cdr_identifier == DDSI_RTPS_SAMPLE_NATIVE);
+  assert (loaned_sample->metadata->cdr_options == 0);
 
   struct dds_serdata_default *d;
-  if (serialize_data)
+  if (will_require_cdr)
   {
-    // maybe if there is a loan and that loan is not the sample, use the loan block as the serialization buffer?
+    // If serialization is/will be required, construct the serdata the normal way
     d = (struct dds_serdata_default *) type->serdata_ops->from_sample (type, kind, sample);
+    if (d == NULL)
+      return NULL;
   }
   else
   {
+    // If we know there is no neeed for the serialized representation (so only PSMX and "memcpy safe"),
+    // construct an empty serdata and stay away from the serializers
     d = serdata_default_new (tp, kind, tp->write_encoding_version);
-    if (d == NULL || !gen_serdata_key_from_sample (tp, &d->key, sample))
+    if (d == NULL)
       return NULL;
-  }
-
-  if (d != NULL)
-  {
-    // now owner of loan
-    d->c.loan = loaned_sample;
-    if (d->c.loan->sample_ptr != sample) //if the sample we are serializing is itself not loaned
-    {
-      assert (d->c.loan->metadata->sample_state == DDS_LOANED_SAMPLE_STATE_UNITIALIZED);
-      if (serialize_data)
-      {
-        d->c.loan->metadata->sample_state = (kind == SDK_KEY ? DDS_LOANED_SAMPLE_STATE_SERIALIZED_KEY : DDS_LOANED_SAMPLE_STATE_SERIALIZED_DATA);
-        d->c.loan->metadata->cdr_identifier = d->hdr.identifier;
-        d->c.loan->metadata->cdr_options = d->hdr.options;
-        memcpy (d->c.loan->sample_ptr, d->data, d->c.loan->metadata->sample_size);
-      }
-      else
-      {
-        d->c.loan->metadata->sample_state = (kind == SDK_KEY ? DDS_LOANED_SAMPLE_STATE_RAW_KEY : DDS_LOANED_SAMPLE_STATE_RAW_DATA);
-        d->c.loan->metadata->cdr_identifier = DDSI_RTPS_SAMPLE_NATIVE;
-        d->c.loan->metadata->cdr_options = 0;
-        memcpy (d->c.loan->sample_ptr, sample, d->c.loan->metadata->sample_size);
-      }
-    }
-    else
+    if (!gen_serdata_key_from_sample (tp, &d->key, sample))
     {
-      d->c.loan->metadata->sample_state = (kind == SDK_KEY ? DDS_LOANED_SAMPLE_STATE_RAW_KEY : DDS_LOANED_SAMPLE_STATE_RAW_DATA);
-      d->c.loan->metadata->cdr_identifier = DDSI_RTPS_SAMPLE_NATIVE;
-      d->c.loan->metadata->cdr_options = 0;
+      ddsi_serdata_unref (&d->c);
+      return NULL;
     }
-
-    if (tp->c.has_key)
-      (void) fix_serdata_default (d, tp->c.serdata_basehash);
-    else
-      (void) fix_serdata_default_nokey (d, tp->c.serdata_basehash);
   }
 
+  // now owner of loan
+  d->c.loan = loaned_sample;
+  if (tp->c.has_key)
+    (void) fix_serdata_default (d, tp->c.serdata_basehash);
+  else
+    (void) fix_serdata_default_nokey (d, tp->c.serdata_basehash);
   return (struct ddsi_serdata *) d;
 }