Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include type-info in sertype_equal for default sertype implementation #1698

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dpotman
Copy link
Contributor

@dpotman dpotman commented May 22, 2023

When comparing sertypes that have xtypes type meta-data, the type information should be included. This PR adds the check to the default sertype implementation and adds a test for this.

When comparing sertypes that have xtypes type meta-data, the type
information should be included. This commit adds the check to the
default sertype implementation and adds a test for this.

Signed-off-by: Dennis Potman <[email protected]>
if (a->type.flagset & DDS_TOPIC_XTYPES_METADATA)
{
ddsi_typeinfo_t *ti_a, *ti_b;
ti_a = ddsi_typeinfo_deser (a->typeinfo_ser.data, a->typeinfo_ser.sz);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to deserialize here? Or can we get away with comparing the serialised representation?

For that, we would have to be certain about the endianness and padding, and perhaps something else. The first two are reasonably easy to deal with, the third is of course tricky 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is the set of hash type identifiers the type depends on, that is included in a type-info object. The spec (7.6.3.2.1) does not specify any ordering for this list (TypeInformation.[minimal/complete].dependent_typeids), and it is also allowed to leave out one or more type identifiers, as long as the dependent type count is correct (TypeInformation.[minimal/complete].dependent_typeid_count).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants