write-like ops: consistent check of data/timestamp

Some functions, notable dds_write_ts rejected negative time stamps, a
choice that I think is reasonable.

Unregister and dispose did not always check, but those occur necessarily
after a write, and therefore allowing negative time stamps if they are
rejected by a write operation makes no sense.  So adding a check here is
a bug fix.

All write-like functions taking a data argument check for a null pointer
before anything else.  dds_writedispose_ts failed to do this, which then
results in a different error code.  That bug is also fixed here.

Signed-off-by: Erik Boasson <[email protected]>

src/core/ddsc/include/dds/dds.h

@@ -2105,7 +2105,7 @@ dds_unregister_instance_ih(dds_entity_t writer, dds_instance_handle_t handle);
  *
  * @param[in]  writer    The writer to which instance is associated.
  * @param[in]  data      The instance with the key value.
- * @param[in]  timestamp The timestamp used at registration.
+ * @param[in]  timestamp The timestamp for the unregistration (>= 0).
  *
  * @returns A dds_return_t indicating success or failure.
  *
@@ -2133,7 +2133,7 @@ dds_unregister_instance_ts(
  *
  * @param[in]  writer    The writer to which instance is associated.
  * @param[in]  handle    The instance handle.
- * @param[in]  timestamp The timestamp used at registration.
+ * @param[in]  timestamp The timestamp for the unregistration (>= 0).
  *
  * @returns A dds_return_t indicating success or failure.
  *
@@ -2217,7 +2217,7 @@ dds_writedispose(dds_entity_t writer, const void *data);
  *
  * @param[in]  writer    The writer to dispose the data instance from.
  * @param[in]  data      The data to be written and disposed.
- * @param[in]  timestamp The timestamp used as source timestamp.
+ * @param[in]  timestamp The timestamp used as source timestamp (>= 0).
  *
  * @returns A dds_return_t indicating success or failure.
  *
@@ -2314,7 +2314,7 @@ dds_dispose(dds_entity_t writer, const void *data);
  * @param[in]  writer    The writer to dispose the data instance from.
  * @param[in]  data      The data sample that identifies the instance
  *                       to be disposed.
- * @param[in]  timestamp The timestamp used as source timestamp.
+ * @param[in]  timestamp The timestamp used as source timestamp (>= 0).
  *
  * @returns A dds_return_t indicating success or failure.
  *
@@ -2392,7 +2392,7 @@ dds_dispose_ih(dds_entity_t writer, dds_instance_handle_t handle);
  *
  * @param[in]  writer    The writer to dispose the data instance from.
  * @param[in]  handle    The handle to identify an instance.
- * @param[in]  timestamp The timestamp used as source timestamp.
+ * @param[in]  timestamp The timestamp used as source timestamp (>= 0).
  *
  * @returns A dds_return_t indicating success or failure.
  *
@@ -2523,7 +2523,7 @@ dds_forwardcdr(dds_entity_t writer, struct ddsi_serdata *serdata);
  *
  * @param[in]  writer The writer entity.
  * @param[in]  data Value to be written.
- * @param[in]  timestamp Source timestamp.
+ * @param[in]  timestamp Source timestamp (>= 0).
  *
  * @returns A dds_return_t indicating success or failure.
  */

src/core/ddsc/src/dds_instance.c

@@ -137,6 +137,9 @@ dds_return_t dds_unregister_instance_ih_ts (dds_entity_t writer, dds_instance_ha
   dds_writer *wr;
   struct ddsi_tkmap_instance *tk;
 
+  if (timestamp < 0)
+    return DDS_RETCODE_BAD_PARAMETER;
+
   if ((ret = dds_writer_lock (writer, &wr)) != DDS_RETCODE_OK)
     return ret;
 
@@ -171,6 +174,9 @@ dds_return_t dds_writedispose_ts (dds_entity_t writer, const void *data, dds_tim
   dds_return_t ret;
   dds_writer *wr;
 
+  if (data == NULL || timestamp < 0)
+    return DDS_RETCODE_BAD_PARAMETER;
+
   if ((ret = dds_writer_lock (writer, &wr)) != DDS_RETCODE_OK)
     return ret;
 
@@ -199,7 +205,7 @@ dds_return_t dds_dispose_ts (dds_entity_t writer, const void *data, dds_time_t t
   dds_return_t ret;
   dds_writer *wr;
 
-  if (data == NULL)
+  if (data == NULL || timestamp < 0)
     return DDS_RETCODE_BAD_PARAMETER;
 
   if ((ret = dds_writer_lock (writer, &wr)) != DDS_RETCODE_OK)
@@ -218,6 +224,9 @@ dds_return_t dds_dispose_ih_ts (dds_entity_t writer, dds_instance_handle_t handl
   dds_return_t ret;
   dds_writer *wr;
 
+  if (timestamp < 0)
+    return DDS_RETCODE_BAD_PARAMETER;
+
   if ((ret = dds_writer_lock (writer, &wr)) != DDS_RETCODE_OK)
     return ret;
 

src/core/ddsc/tests/dispose.c

@@ -177,9 +177,11 @@ CU_TheoryDataPoints(ddsc_writedispose, non_writers) = {
 };
 CU_Theory((dds_entity_t *writer), ddsc_writedispose, non_writers, .init=disposing_init, .fini=disposing_fini)
 {
+    Space_Type1 data = { 0, 22, 22 };
     dds_return_t ret;
+
     DDSRT_WARNING_MSVC_OFF(6387); /* Disable SAL warning on intentional misuse of the API */
-    ret = dds_writedispose(*writer, NULL);
+    ret = dds_writedispose(*writer, &data);
     DDSRT_WARNING_MSVC_ON(6387);
     CU_ASSERT_EQUAL_FATAL(ret, DDS_RETCODE_ILLEGAL_OPERATION);
 }
@@ -347,9 +349,10 @@ CU_TheoryDataPoints(ddsc_writedispose_ts, non_writers) = {
 };
 CU_Theory((dds_entity_t *writer), ddsc_writedispose_ts, non_writers, .init=disposing_init, .fini=disposing_fini)
 {
+    Space_Type1 data = { 0, 22, 22 };
     dds_return_t ret;
     DDSRT_WARNING_MSVC_OFF(6387); /* Disable SAL warning on intentional misuse of the API */
-    ret = dds_writedispose_ts(*writer, NULL, g_present);
+    ret = dds_writedispose_ts(*writer, &data, g_present);
     DDSRT_WARNING_MSVC_ON(6387);
     CU_ASSERT_EQUAL_FATAL(ret, DDS_RETCODE_ILLEGAL_OPERATION);
 }