Bug report rosidl_runtime_c__U16String__

Inconsistent APIs between String and U16String about rosidl HOT 1 OPEN

allsey87 commented on September 28, 2024

Inconsistent APIs between String and U16String

from rosidl.

Comments (1)

sloretz commented on September 28, 2024

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); 

 }

from rosidl.

Inconsistent APIs between String and U16String about rosidl HOT 1 OPEN

Comments (1)

Related Issues (20)

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	/// 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);

	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);
	}