Inconsistent APIs between String and U16String

[allsey87]

Bug report

rosidl_runtime_c__U16String__resize exists, however, there is no rosidl_runtime_c__String__resize.

Required Info:

• Version or commit hash: Rolling

Expected behavior

Consistent APIs between String and U16String

Actual behavior

Inconsistent APIs between String and U16String

Feature request

Make APIs consistent?

Feature description

Sometimes a char * is not directly available, e.g., from a deserialization library. This means that it is not possible to use assignn without allocating an intermediate buffer. Having a resize function for both String and U16String would solve this problem.

Implementation considerations

The workaround I will use is to use rcutils_get_default_allocator to update the internals of the string manually.

[sloretz]

Thank you for the report! It looks like a resize function is missing. There's a resize_assignn test, but it doesn't resize an existing string. I'll leave this open as a help wanted. Fixing it means doing the following:

Make a declaration similar to this one, but using char instead of uint16_t and put it in string_functions.h

rosidl/rosidl_runtime_c/include/rosidl_runtime_c/u16string_functions.h

Lines 156 to 167 in d66b6db

	/// Resize the uint16_t pointer.
	/*
	* This function resize the input value pointer.
	*
	* \param[in] n the new size of the internal buffer
	* \return true if successful, false if the passed string pointer is null
	* or if the size is higher than SIZE_MAX or if the memory reallocation failed.
	*/
	ROSIDL_GENERATOR_C_PUBLIC
	bool
	rosidl_runtime_c__U16String__resize(
	rosidl_runtime_c__U16String * str, size_t n);

Implement it similar to this function, but using char instead of uint16_t and put it in string_functions.c

rosidl/rosidl_runtime_c/src/u16string_functions.c

Lines 167 to 188 in d66b6db

	bool
	rosidl_runtime_c__U16String__resize(
	rosidl_runtime_c__U16String * str, size_t n)
	{
	if (!str) {
	return false;
	}
	// check valid range of n before allocating n + 1 characters
	if (n > SIZE_MAX / sizeof(uint16_t) - 1) {
	return false;
	}
	rcutils_allocator_t allocator = rcutils_get_default_allocator();
	uint16_t * data = allocator.reallocate(str->data, (n + 1) * sizeof(uint16_t), allocator.state);
	if (!data) {
	return false;
	}
	data[n] = 0;
	str->data = data;
	str->size = n;
	str->capacity = n + 1;
	return true;
	}

Take the expectations on U16String_resize from the test below, and put them into into test_string_functions.cpp for the new function.

rosidl/rosidl_runtime_c/test/test_u16string_functions.cpp

Lines 73 to 147 in d66b6db

	TEST(u16string_functions, resize_assignn) {
	rosidl_runtime_c__U16String s, t;
	constexpr size_t s_size = 3u;
	constexpr uint16_t data[s_size + 1] = {3u, 2u, 1u, 0u};
	EXPECT_TRUE(rosidl_runtime_c__U16String__init(&s));
	EXPECT_TRUE(rosidl_runtime_c__U16String__init(&t));
	EXPECT_FALSE(rosidl_runtime_c__U16String__resize(nullptr, s_size));
	// If you're here because this test crashed your computer, it might be because it just tried
	// to allocate SIZE_MAX * 2 bytes, which means someone removed an important check.
	EXPECT_FALSE(rosidl_runtime_c__U16String__resize(&s, SIZE_MAX));
	EXPECT_TRUE(rosidl_runtime_c__U16String__resize(&s, s_size));
	EXPECT_EQ(s.size, s_size);
	EXPECT_EQ(s.capacity, s_size + 1);
	EXPECT_NE(s.data, nullptr);
	EXPECT_EQ(s.data[s_size], 0u);
	EXPECT_FALSE(rosidl_runtime_c__U16String__assign(nullptr, nullptr));
	EXPECT_FALSE(rosidl_runtime_c__U16String__assignn(nullptr, nullptr, 0));
	EXPECT_FALSE(rosidl_runtime_c__U16String__assignn(&s, nullptr, 0));
	EXPECT_FALSE(rosidl_runtime_c__U16String__assignn(nullptr, &data[0], 0));
	EXPECT_TRUE(rosidl_runtime_c__U16String__assign(&t, &data[0]));
	EXPECT_EQ(t.size, s_size);
	EXPECT_EQ(t.capacity, s_size + 1u);
	EXPECT_EQ(t.data[0], data[0]);
	EXPECT_EQ(t.data[1], data[1]);
	EXPECT_EQ(t.data[2], data[2]);
	EXPECT_EQ(t.data[3], 0u);
	EXPECT_TRUE(rosidl_runtime_c__U16String__assignn(&s, &data[0], s_size));
	EXPECT_EQ(s.size, s_size);
	EXPECT_EQ(s.capacity, s_size + 1u);
	EXPECT_EQ(s.data[0], data[0]);
	EXPECT_EQ(s.data[1], data[1]);
	EXPECT_EQ(s.data[2], data[2]);
	EXPECT_EQ(s.data[3], 0u);
	// Can't copy strings of size max
	EXPECT_FALSE(rosidl_runtime_c__U16String__assignn(&s, &data[0], SIZE_MAX));
	// Check assigning 0-length strings
	EXPECT_TRUE(rosidl_runtime_c__U16String__assignn(&s, &data[0], 0));
	EXPECT_EQ(s.size, 0u);
	EXPECT_EQ(s.capacity, 1u);
	EXPECT_EQ(s.data[0], 0u);
	constexpr size_t s8_size = 2 * s_size;
	// To avoid endian issues, these double-wide chars are the same for big/little endian
	constexpr char data_8t[s8_size + 2u] = {3, 3, 2, 2, 1, 1, 0, 0};
	// str is nullptr
	EXPECT_FALSE(rosidl_runtime_c__U16String__assignn_from_char(nullptr, nullptr, 0));
	// value is nullptr
	EXPECT_FALSE(rosidl_runtime_c__U16String__assignn_from_char(&s, nullptr, 0));
	// n is odd
	EXPECT_FALSE(rosidl_runtime_c__U16String__assignn_from_char(&s, &data_8t[0], 1u));
	EXPECT_TRUE(rosidl_runtime_c__U16String__assignn_from_char(&s, &data_8t[0], s8_size));
	EXPECT_EQ(s.size, s_size);
	EXPECT_EQ(s.capacity, s_size + 1u);
	EXPECT_EQ(s.data[0], 771u);
	EXPECT_EQ(s.data[1], 514u);
	EXPECT_EQ(s.data[2], 257u);
	EXPECT_EQ(s.data[3], 0u);
	rosidl_runtime_c__U16String__fini(&s);
	rosidl_runtime_c__U16String__fini(&t);
	// Check assign after fini without init
	EXPECT_TRUE(rosidl_runtime_c__U16String__assign(&s, &data[0]));
	rosidl_runtime_c__U16String__fini(&s);
	}