Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Write-like operations: consistent check of data/timestamp #1890

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/core/ddsc/include/dds/dds.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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.
*/
Expand Down
11 changes: 10 additions & 1 deletion src/core/ddsc/src/dds_instance.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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)
Expand All @@ -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;

Expand Down
7 changes: 5 additions & 2 deletions src/core/ddsc/tests/dispose.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nitpick: I think these (this one and below) disable-warnings can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. I think there are several more of those, I'll do that in a separate PR.

ret = dds_writedispose(*writer, NULL);
ret = dds_writedispose(*writer, &data);
DDSRT_WARNING_MSVC_ON(6387);
CU_ASSERT_EQUAL_FATAL(ret, DDS_RETCODE_ILLEGAL_OPERATION);
}
Expand Down Expand Up @@ -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);
}
Expand Down