From 6ee69374ec67d58af3fdab2d744774ec35140a55 Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Thu, 30 Apr 2020 09:20:04 +0200 Subject: [PATCH] Standard byte order when creating built-in samples The standard defines GUIDs as an array of 16 uint8_t and so they are presented on the network and in built-in topic samples. Internally they are arrays of 4 uint32_t, requiring byte-order conversion. A keyhash is also an array of 16 uint8_t, and used almost exclusively on the network. The exception is the generation of built-in topic samples, which relies on the "from_keyhash" function. One would expect the keyhash here to contain the GUID in the external representation, and this commit adds the byte-order conversions to conform to the expectation. Signed-off-by: Erik Boasson --- src/core/ddsc/src/dds_builtin.c | 18 +++++++++++------- src/core/ddsc/src/dds_serdata_builtintopic.c | 10 ++++++---- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/core/ddsc/src/dds_builtin.c b/src/core/ddsc/src/dds_builtin.c index eee50f1..d224810 100644 --- a/src/core/ddsc/src/dds_builtin.c +++ b/src/core/ddsc/src/dds_builtin.c @@ -15,6 +15,7 @@ #include "dds/ddsi/q_entity.h" #include "dds/ddsi/q_thread.h" #include "dds/ddsi/q_config.h" +#include "dds/ddsi/q_bswap.h" #include "dds/ddsi/ddsi_domaingv.h" #include "dds/ddsi/ddsi_plist.h" #include "dds__init.h" @@ -181,10 +182,13 @@ static struct ddsi_tkmap_instance *dds__builtin_get_tkmap_entry (const struct dd struct dds_domain *domain = vdomain; struct ddsi_tkmap_instance *tk; struct ddsi_serdata *sd; - struct ddsi_keyhash kh; - memcpy (&kh, guid, sizeof (kh)); - /* any random builtin topic will do (provided it has a GUID for a key), because what matters is the "class" of the topic, not the actual topic; also, this is called early in the initialisation of the entity with this GUID, which simply causes serdata_from_keyhash to create a key-only serdata because the key lookup fails. */ - sd = ddsi_serdata_from_keyhash (domain->builtin_participant_topic, &kh); + union { ddsi_guid_t guid; struct ddsi_keyhash keyhash; } x; + x.guid = nn_hton_guid (*guid); + /* any random builtin topic will do (provided it has a GUID for a key), because what matters is the "class" + of the topic, not the actual topic; also, this is called early in the initialisation of the entity with + this GUID, which simply causes serdata_from_keyhash to create a key-only serdata because the key lookup + fails. */ + sd = ddsi_serdata_from_keyhash (domain->builtin_participant_topic, &x.keyhash); tk = ddsi_tkmap_find (domain->gv.m_tkmap, sd, true); ddsi_serdata_unref (sd); return tk; @@ -196,7 +200,7 @@ struct ddsi_serdata *dds__builtin_make_sample (const struct entity_common *e, dd struct dds_domain *dom = e->gv->builtin_topic_interface->arg; struct ddsi_sertopic *topic = NULL; struct ddsi_serdata *serdata; - struct ddsi_keyhash keyhash; + union { ddsi_guid_t guid; struct ddsi_keyhash keyhash; } x; switch (e->kind) { case EK_PARTICIPANT: @@ -213,8 +217,8 @@ struct ddsi_serdata *dds__builtin_make_sample (const struct entity_common *e, dd break; } assert (topic != NULL); - memcpy (&keyhash, &e->guid, sizeof (keyhash)); - serdata = ddsi_serdata_from_keyhash (topic, &keyhash); + x.guid = nn_hton_guid (e->guid); + serdata = ddsi_serdata_from_keyhash (topic, &x.keyhash); serdata->timestamp = timestamp; serdata->statusinfo = alive ? 0 : NN_STATUSINFO_DISPOSE | NN_STATUSINFO_UNREGISTER; return serdata; diff --git a/src/core/ddsc/src/dds_serdata_builtintopic.c b/src/core/ddsc/src/dds_serdata_builtintopic.c index 614a566..9991e8b 100644 --- a/src/core/ddsc/src/dds_serdata_builtintopic.c +++ b/src/core/ddsc/src/dds_serdata_builtintopic.c @@ -130,10 +130,12 @@ static struct ddsi_serdata *ddsi_serdata_builtin_from_keyhash (const struct ddsi { /* FIXME: not quite elegant to manage the creation of a serdata for a built-in topic via this function, but I also find it quite unelegant to let from_sample read straight from the underlying internal entity, and to_sample convert to the external format ... I could claim the internal entity is the "serialised form", but that forces wrapping it in a fragchain in one way or another, which, though possible, is also a bit lacking in elegance. */ const struct ddsi_sertopic_builtintopic *tp = (const struct ddsi_sertopic_builtintopic *)tpcmn; - /* keyhash must in host format (which the GUIDs always are internally) */ - struct entity_common *entity = entidx_lookup_guid_untyped (tp->c.gv->entity_index, (const ddsi_guid_t *) keyhash->value); - struct ddsi_serdata_builtintopic *d = serdata_builtin_new(tp, entity ? SDK_DATA : SDK_KEY); - memcpy (&d->key, keyhash->value, sizeof (d->key)); + union { ddsi_guid_t guid; ddsi_keyhash_t keyhash; } x; + x.keyhash = *keyhash; + x.guid = nn_ntoh_guid (x.guid); + struct entity_common *entity = entidx_lookup_guid_untyped (tp->c.gv->entity_index, &x.guid); + struct ddsi_serdata_builtintopic *d = serdata_builtin_new (tp, entity ? SDK_DATA : SDK_KEY); + d->key = x.guid; if (entity) { ddsrt_mutex_lock (&entity->qos_lock);