From 0a7e5ff8eb18d6f6a0f183a69c82c09baabce086 Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Sun, 5 May 2019 11:22:14 +0800 Subject: [PATCH] remove name arg from dds_create_topic_arbitrary The name parameter and the name in the sertopic parameter had to match because it used the one as a key in a lookup checking whether the topic exists already, and the other as key for the nodes in that index. As the name is (currently) included in the sertopic, it shouldn't be passed in separately as well. Signed-off-by: Erik Boasson --- src/core/ddsc/include/dds/dds.h | 4 +--- src/core/ddsc/src/dds_builtin.c | 2 +- src/core/ddsc/src/dds_topic.c | 29 +++++++++++++---------------- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/core/ddsc/include/dds/dds.h b/src/core/ddsc/include/dds/dds.h index d701117..07b1545 100644 --- a/src/core/ddsc/include/dds/dds.h +++ b/src/core/ddsc/include/dds/dds.h @@ -967,8 +967,7 @@ dds_create_topic( * matching is done on a combination of topic name and type name. * * @param[in] participant Participant on which to create the topic. - * @param[in] sertopic Internal description of the topic type. - * @param[in] name Name of the topic. + * @param[in] sertopic Internal description of the topic type (includes name). * @param[in] qos QoS to set on the new topic (can be NULL). * @param[in] listener Any listener functions associated with the new topic (can be NULL). * @param[in] sedp_plist Topic description to be published as part of discovery (if NULL, not published). @@ -987,7 +986,6 @@ DDS_EXPORT dds_entity_t dds_create_topic_arbitrary ( dds_entity_t participant, struct ddsi_sertopic *sertopic, - const char *name, const dds_qos_t *qos, const dds_listener_t *listener, const struct nn_plist *sedp_plist); diff --git a/src/core/ddsc/src/dds_builtin.c b/src/core/ddsc/src/dds_builtin.c index 3f50c60..fff187a 100644 --- a/src/core/ddsc/src/dds_builtin.c +++ b/src/core/ddsc/src/dds_builtin.c @@ -97,7 +97,7 @@ dds_entity_t dds__get_builtin_topic (dds_entity_t e, dds_entity_t topic) } dds_qos_t *qos = dds__create_builtin_qos (); - tp = dds_create_topic_arbitrary (pp, sertopic, sertopic->name, qos, NULL, NULL); + tp = dds_create_topic_arbitrary (pp, sertopic, qos, NULL, NULL); dds_delete_qos (qos); return tp; } diff --git a/src/core/ddsc/src/dds_topic.c b/src/core/ddsc/src/dds_topic.c index 747976c..c0cb41d 100644 --- a/src/core/ddsc/src/dds_topic.c +++ b/src/core/ddsc/src/dds_topic.c @@ -191,6 +191,11 @@ dds_find_topic( ddsrt_mutex_lock (&dds_global.m_mutex); st = dds_topic_lookup_locked (p->m_domain, name); if (st) { + /* FIXME: calling addref is wrong because the Cyclone library has no + knowledge of the reference and hence simply deleting the participant + won't make the ref count drop to 0. On the other hand, the DDS spec + says find_topic (and a second call to create_topic) return a new + proxy that must separately be deleted. */ dds_entity_add_ref (&st->status_cb_entity->m_entity); tp = st->status_cb_entity->m_entity.m_hdllink.hdl; } else { @@ -300,7 +305,6 @@ DDS_EXPORT dds_entity_t dds_create_topic_arbitrary ( dds_entity_t participant, struct ddsi_sertopic *sertopic, - const char *name, const dds_qos_t *qos, const dds_listener_t *listener, const nn_plist_t *sedp_plist) @@ -319,18 +323,6 @@ dds_create_topic_arbitrary ( goto bad_param_err; } - if (name == NULL) { - DDS_ERROR("Topic name is NULL\n"); - hdl = DDS_ERRNO(DDS_RETCODE_BAD_PARAMETER); - goto bad_param_err; - } - - if (!is_valid_name(name)) { - DDS_ERROR("Topic name contains characters that are not allowed\n"); - hdl = DDS_ERRNO(DDS_RETCODE_BAD_PARAMETER); - goto bad_param_err; - } - rc = dds_entity_lock(participant, DDS_KIND_PARTICIPANT, &par); if (rc != DDS_RETCODE_OK) { hdl = DDS_ERRNO(rc); @@ -349,8 +341,8 @@ dds_create_topic_arbitrary ( /* Check if topic already exists with same name */ ddsrt_mutex_lock (&dds_global.m_mutex); - if ((stgeneric = dds_topic_lookup_locked (par->m_domain, name)) != NULL) { - if (!sertopic_equivalent (stgeneric,sertopic)) { + if ((stgeneric = dds_topic_lookup_locked (par->m_domain, sertopic->name)) != NULL) { + if (!sertopic_equivalent (stgeneric, sertopic)) { /* FIXME: should copy the type, perhaps? but then the pointers will no longer be the same */ DDS_ERROR("Create topic with mismatching type\n"); hdl = DDS_ERRNO(DDS_RETCODE_PRECONDITION_NOT_MET); @@ -359,6 +351,11 @@ dds_create_topic_arbitrary ( DDS_ERROR("Create topic with mismatching qos\n"); hdl = DDS_ERRNO(DDS_RETCODE_INCONSISTENT_POLICY); } else { + /* FIXME: calling addref is wrong because the Cyclone library has no + knowledge of the reference and hence simply deleting the participant + won't make the ref count drop to 0. On the other hand, the DDS spec + says find_topic (and a second call to create_topic) return a new + proxy that must separately be deleted. */ dds_entity_add_ref (&stgeneric->status_cb_entity->m_entity); hdl = stgeneric->status_cb_entity->m_entity.m_hdllink.hdl; } @@ -487,7 +484,7 @@ dds_create_topic( } } - hdl = dds_create_topic_arbitrary(participant, &st->c, name, qos, listener, &plist); + hdl = dds_create_topic_arbitrary(participant, &st->c, qos, listener, &plist); ddsi_sertopic_unref (&st->c); nn_plist_fini (&plist);