From 6dc28db19766ec6a266839bbfcce89cab60daf4f Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Sat, 22 Feb 2020 10:51:08 +0100 Subject: [PATCH] Fix warning by cleaning up dds_set_qos_locked_raw gcc 5.4 correctly warned that a null pointer was being passed into the entity-specific "set_qos" function when changing a topic QoS, where that parameter was tagged as "non-null". As it was never dereferenced in this case the resulting behaviour was still correct. It turns out that the entire function was overly complicated and that simply passing the entity pointer round allows eliminating a few arguments as well. (Oddly none of the more modern toolchains used pick this up.) Signed-off-by: Erik Boasson --- src/core/ddsc/src/dds_entity.c | 13 ++++++------- src/core/ddsc/src/dds_topic.c | 4 +--- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/core/ddsc/src/dds_entity.c b/src/core/ddsc/src/dds_entity.c index 2b842af..e6620db 100644 --- a/src/core/ddsc/src/dds_entity.c +++ b/src/core/ddsc/src/dds_entity.c @@ -677,8 +677,9 @@ dds_return_t dds_get_qos (dds_entity_t entity, dds_qos_t *qos) return ret; } -static dds_return_t dds_set_qos_locked_raw (dds_entity *e, dds_qos_t **e_qos_ptr, bool e_enabled, const dds_qos_t *qos, uint64_t mask, const struct ddsrt_log_cfg *logcfg, dds_return_t (*set_qos) (struct dds_entity *e, const dds_qos_t *qos, bool enabled) ddsrt_nonnull_all) +static dds_return_t dds_set_qos_locked_raw (dds_entity *e, dds_qos_t **e_qos_ptr, const dds_qos_t *qos, uint64_t mask, const struct ddsrt_log_cfg *logcfg) { + const bool enabled = ((e->m_flags & DDS_ENTITY_ENABLED) != 0); dds_return_t ret; /* Any attempt to do this on a topic ends up doing it on the ktopic instead, so that there is @@ -692,7 +693,7 @@ static dds_return_t dds_set_qos_locked_raw (dds_entity *e, dds_qos_t **e_qos_ptr /* invalid or inconsistent QoS settings */ goto error_or_nochange; } - else if (!e_enabled) + else if (!enabled) { /* do as you please while the entity is not enabled */ } @@ -728,7 +729,7 @@ static dds_return_t dds_set_qos_locked_raw (dds_entity *e, dds_qos_t **e_qos_ptr } assert (ret == DDS_RETCODE_OK); - if ((ret = set_qos (e, newqos, e_enabled)) != DDS_RETCODE_OK) + if ((ret = dds_entity_deriver_set_qos (e, newqos, enabled)) != DDS_RETCODE_OK) goto error_or_nochange; else { @@ -748,7 +749,7 @@ static dds_return_t dds_set_qos_locked_impl (dds_entity *e, const dds_qos_t *qos dds_entity_kind_t kind = dds_entity_kind (e); if (kind != DDS_KIND_TOPIC) { - return dds_set_qos_locked_raw (e, &e->m_qos, (e->m_flags & DDS_ENTITY_ENABLED) != 0, qos, mask, logcfg, dds_entity_deriver_table[kind]->set_qos); + return dds_set_qos_locked_raw (e, &e->m_qos, qos, mask, logcfg); } else { @@ -767,9 +768,7 @@ static dds_return_t dds_set_qos_locked_impl (dds_entity *e, const dds_qos_t *qos while (ktp->defer_set_qos != 0) ddsrt_cond_wait (&pp->m_entity.m_cond, &pp->m_entity.m_mutex); - /* dds_entity_deriver_table[kind]->set_qos had better avoid looking at the entity! */ - rc = dds_set_qos_locked_raw (NULL, &ktp->qos, (e->m_flags & DDS_ENTITY_ENABLED) != 0, qos, mask, logcfg, dds_entity_deriver_table[kind]->set_qos); - + rc = dds_set_qos_locked_raw (e, &ktp->qos, qos, mask, logcfg); ddsrt_mutex_unlock (&pp->m_entity.m_mutex); return rc; } diff --git a/src/core/ddsc/src/dds_topic.c b/src/core/ddsc/src/dds_topic.c index 93db04a..79a4eee 100644 --- a/src/core/ddsc/src/dds_topic.c +++ b/src/core/ddsc/src/dds_topic.c @@ -170,10 +170,8 @@ static dds_return_t dds_topic_qos_set (dds_entity *e, const dds_qos_t *qos, bool { /* We never actually set the qos of a struct dds_topic and really shouldn't be here, but the code to check whether set_qos is supported uses the entity's qos_set - function as a proxy. One of the weird things about the topic's set_qos is that - this is called with e == NULL. */ + function as a proxy. */ (void) e; (void) qos; (void) enabled; - assert (e == NULL); return DDS_RETCODE_OK; }