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.

Sign up for GitHub

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

Jump to bottom

dynsub: fix bounded strings #1812

Merged
merged 1 commit into from
Aug 28, 2023
Merged

dynsub: fix bounded strings #1812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions examples/dynsub/print_sample.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,15 @@ static void print_sample1_ti (const unsigned char *sample, const DDS_XTypes_Type
{
case DDS_XTypes_TI_STRING8_SMALL:
case DDS_XTypes_TI_STRING8_LARGE: {
const char **p = (const char **) align (sample, c, _Alignof (char *), sizeof (char *));
const char *p = align (sample, c, _Alignof (char *), sizeof (char *));
if (c->key || c->valid_data)
{
printf ("%s", sep);
if (label) printf ("\"%s\":", label);
printf ("\"%s\"", *p);
if ((typeid->_d == DDS_XTypes_TI_PLAIN_SEQUENCE_SMALL) ? typeid->_u.string_sdefn.bound : typeid->_u.string_ldefn.bound)
printf ("\"%s\"", p);
else
printf ("\"%s\"", *((const char **) p));
Comment on lines +90 to +92
Copy link
Contributor

Choose a reason for hiding this comment

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

I wanted to clarify my comprehension that this is involving some sort of small string/sequence optimization where the characters are immediately available at p for small strings but requires pointer indirection otherwise?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Someone way back when (not me, I just inherited it) chose to map bounded strings to inlined arrays of characters, even though the IDL-to-C mapping specifies it is a pointer just like an unbounded string.

There is a reasonable argument: this is one of very few decent ways to support bounded strings without requiring memory allocation on reading samples. You can do preallocation of memory for sequences, structs, arrays, unions, ... anything, but not for strings in the official mapping. So it makes sense.

With XTypes, you can declare it @external and you'll get a pointer anyway if you really want. (dynsub isn't supporting that (yet).)

}
break;
}
Expand Down
15 changes: 13 additions & 2 deletions examples/dynsub/type_cache.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,19 @@ static void build_typecache_ti (const DDS_XTypes_TypeIdentifier *typeid, size_t
{
case DDS_XTypes_TI_STRING8_SMALL:
case DDS_XTypes_TI_STRING8_LARGE: {
*align = _Alignof (unsigned char *);
*size = sizeof (unsigned char *);
uint32_t bound;
if (typeid->_d == DDS_XTypes_TI_PLAIN_SEQUENCE_SMALL) {
bound = typeid->_u.string_sdefn.bound;
} else {
bound = typeid->_u.string_ldefn.bound;
}
if (bound == 0) {
*align = _Alignof (unsigned char *);
*size = sizeof (unsigned char *);
} else {
*align = 1;
*size = bound;
}
break;
}
case DDS_XTypes_TI_PLAIN_SEQUENCE_SMALL:
Expand Down
Loading