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

Standard byte order when creating built-in samples

Browse source 
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 <eb@ilities.com>
This commit is contained in:
Erik Boasson 2020-04-30 09:20:04 +02:00 • committed by eboasson
parent ebdb3fc5cf
commit 6ee69374ec
2 changed files with 17 additions and 11 deletions
Download patch file Download diff file Expand all files Collapse all files

18
src/core/ddsc/src/dds_builtin.c
View file

@ -15,6 +15,7 @@
#include "dds/ddsi/q_entity.h" #include "dds/ddsi/q_entity.h"
#include "dds/ddsi/q_thread.h" #include "dds/ddsi/q_thread.h"
#include "dds/ddsi/q_config.h" #include "dds/ddsi/q_config.h"
#include "dds/ddsi/q_bswap.h"
#include "dds/ddsi/ddsi_domaingv.h" #include "dds/ddsi/ddsi_domaingv.h"
#include "dds/ddsi/ddsi_plist.h" #include "dds/ddsi/ddsi_plist.h"
#include "dds__init.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 dds_domain *domain = vdomain;
struct ddsi_tkmap_instance *tk; struct ddsi_tkmap_instance *tk;
struct ddsi_serdata *sd; struct ddsi_serdata *sd;
struct ddsi_keyhash kh; union { ddsi_guid_t guid; struct ddsi_keyhash keyhash; } x;
memcpy (&kh, guid, sizeof (kh)); 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. */ /* any random builtin topic will do (provided it has a GUID for a key), because what matters is the "class"
sd = ddsi_serdata_from_keyhash (domain->builtin_participant_topic, &kh); 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); tk = ddsi_tkmap_find (domain->gv.m_tkmap, sd, true);
ddsi_serdata_unref (sd); ddsi_serdata_unref (sd);
return tk; 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 dds_domain *dom = e->gv->builtin_topic_interface->arg;
struct ddsi_sertopic *topic = NULL; struct ddsi_sertopic *topic = NULL;
struct ddsi_serdata *serdata; struct ddsi_serdata *serdata;
struct ddsi_keyhash keyhash; union { ddsi_guid_t guid; struct ddsi_keyhash keyhash; } x;
switch (e->kind) switch (e->kind)
{ {
case EK_PARTICIPANT: case EK_PARTICIPANT:
@ -213,8 +217,8 @@ struct ddsi_serdata *dds__builtin_make_sample (const struct entity_common *e, dd
break; break;
} }
assert (topic != NULL); assert (topic != NULL);
memcpy (&keyhash, &e->guid, sizeof (keyhash)); x.guid = nn_hton_guid (e->guid);
serdata = ddsi_serdata_from_keyhash (topic, &keyhash); serdata = ddsi_serdata_from_keyhash (topic, &x.keyhash);
serdata->timestamp = timestamp; serdata->timestamp = timestamp;
serdata->statusinfo = alive ? 0 : NN_STATUSINFO_DISPOSE | NN_STATUSINFO_UNREGISTER; serdata->statusinfo = alive ? 0 : NN_STATUSINFO_DISPOSE | NN_STATUSINFO_UNREGISTER;
return serdata; return serdata;

8
src/core/ddsc/src/dds_serdata_builtintopic.c
View file

@ -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. */ /* 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; const struct ddsi_sertopic_builtintopic *tp = (const struct ddsi_sertopic_builtintopic *)tpcmn;
/* keyhash must in host format (which the GUIDs always are internally) */ union { ddsi_guid_t guid; ddsi_keyhash_t keyhash; } x;
struct entity_common *entity = entidx_lookup_guid_untyped (tp->c.gv->entity_index, (const ddsi_guid_t *) keyhash->value); 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); struct ddsi_serdata_builtintopic *d = serdata_builtin_new (tp, entity ? SDK_DATA : SDK_KEY);
memcpy (&d->key, keyhash->value, sizeof (d->key)); d->key = x.guid;
if (entity) if (entity)
{ {
ddsrt_mutex_lock (&entity->qos_lock); ddsrt_mutex_lock (&entity->qos_lock);