niklas/cyclonedds
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix warning by cleaning up dds_set_qos_locked_raw

Browse source 
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 <eb@ilities.com>
This commit is contained in:
Erik Boasson 2020-02-22 10:51:08 +01:00 • committed by eboasson
parent 8bd6f34f67
commit 6dc28db197
2 changed files with 7 additions and 10 deletions
Download patch file Download diff file Expand all files Collapse all files

13
src/core/ddsc/src/dds_entity.c
View file

@ -677,8 +677,9 @@ dds_return_t dds_get_qos (dds_entity_t entity, dds_qos_t *qos)
return ret; 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; dds_return_t ret;
/* Any attempt to do this on a topic ends up doing it on the ktopic instead, so that there is /* 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 */ /* invalid or inconsistent QoS settings */
goto error_or_nochange; goto error_or_nochange;
} }
else if (!e_enabled) else if (!enabled)
{ {
/* do as you please while the entity is not 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); 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; goto error_or_nochange;
else 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); dds_entity_kind_t kind = dds_entity_kind (e);
if (kind != DDS_KIND_TOPIC) 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 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) while (ktp->defer_set_qos != 0)
ddsrt_cond_wait (&pp->m_entity.m_cond, &pp->m_entity.m_mutex); 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 (e, &ktp->qos, qos, mask, logcfg);
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);
ddsrt_mutex_unlock (&pp->m_entity.m_mutex); ddsrt_mutex_unlock (&pp->m_entity.m_mutex);
return rc; return rc;
} }

4
src/core/ddsc/src/dds_topic.c
View file

@ -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, /* 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 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 function as a proxy. */
this is called with e == NULL. */
(void) e; (void) qos; (void) enabled; (void) e; (void) qos; (void) enabled;
assert (e == NULL);
return DDS_RETCODE_OK; return DDS_RETCODE_OK;
} }