Skip to content

Commit cd7968e

Browse files
MiguelCompanyMiguelCompany
authored
Allow zero-valued InstanceHandle_t (#2316)
* Refs 12862. Changed test to use zero-valued valid key. Signed-off-by: Miguel Company <[email protected]> * Refs 12862. Added KeyHash_t. Signed-off-by: Miguel Company <[email protected]> * Refs 12862. Added InstanceHandleValue_t. Signed-off-by: Miguel Company <[email protected]> * Refs 12862. Using InstanceHandleValue_t on InstanceHandle_t. Signed-off-by: Miguel Company <[email protected]> * Refs 12862. Comparison operators on InstanceHandleValue_t. Signed-off-by: Miguel Company <[email protected]> * Refs 12862. Added noexcept where appropiate. Signed-off-by: Miguel Company <[email protected]> * Refs 12942. Using empty initializer list. Signed-off-by: Miguel Company <[email protected]> * Refs 12942. Doxygen on InstanceHandleValue_t. Signed-off-by: Miguel Company <[email protected]> * Refs 12942. Including change on versions.md. Signed-off-by: Miguel Company <[email protected]>
1 parent e3b56eb commit cd7968e

File tree

3 files changed

+143
-103
lines changed

3 files changed

+143
-103
lines changed

‎include/fastdds/rtps/common/InstanceHandle.h

Lines changed: 134 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
#ifndef _FASTDDS_RTPS_INSTANCEHANDLE_H_
2020
#define _FASTDDS_RTPS_INSTANCEHANDLE_H_
2121

22+
#include <array>
23+
2224
#include <fastrtps/fastrtps_dll.h>
2325
#include <fastdds/rtps/common/Types.h>
2426
#include <fastdds/rtps/common/Guid.h>
@@ -27,101 +29,164 @@ namespace eprosima {
2729
namespace fastrtps {
2830
namespace rtps {
2931

32+
using KeyHash_t = std::array<octet, 16>;
33+
34+
struct RTPS_DllAPI InstanceHandleValue_t
35+
{
36+
/**
37+
* Write access indexing operator.
38+
*
39+
* Provides a reference to the byte value at position @c i.
40+
*
41+
* @param [in] i index of the byte to return.
42+
*
43+
* @post Method has_been_set() returns @c true.
44+
*
45+
* @remark Do not use this method to check if this value has been set.
46+
* Use method has_been_set() instead.
47+
*/
48+
octet& operator [] (
49+
size_t i) noexcept
50+
{
51+
has_been_set_ = true;
52+
return value_[i];
53+
}
54+
55+
/**
56+
* Read access indexing operator.
57+
*
58+
* Provides the byte value at position @c i.
59+
*
60+
* @param [in] i index of the byte to return.
61+
*
62+
* @remark Do not use this method to check if this value has been set.
63+
* Use method has_been_set() instead.
64+
*/
65+
octet operator [] (
66+
size_t i) const noexcept
67+
{
68+
return value_[i];
69+
}
70+
71+
/**
72+
* Write access pointer cast operator.
73+
*
74+
* Provides a pointer to the start of the raw data.
75+
*
76+
* @post Method has_been_set() returns @c true.
77+
*
78+
* @remark Do not use this method to check if this value has been set.
79+
* Use method has_been_set() instead.
80+
*/
81+
operator octet* () noexcept
82+
{
83+
has_been_set_ = true;
84+
return value_.data();
85+
}
86+
87+
/**
88+
* Read access pointer cast operator.
89+
*
90+
* Provides a pointer to the start of the raw data.
91+
*
92+
* @remark Do not use this method to check if this value has been set.
93+
* Use method has_been_set() instead.
94+
*/
95+
operator const octet* () const noexcept
96+
{
97+
return value_.data();
98+
}
99+
100+
/**
101+
* Return whether any of the write access operators of this value has been used.
102+
*/
103+
bool has_been_set() const noexcept
104+
{
105+
return has_been_set_;
106+
}
107+
108+
/**
109+
* Equality comparison operator.
110+
*/
111+
bool operator == (
112+
const InstanceHandleValue_t& other) const noexcept
113+
{
114+
return (has_been_set_ == other.has_been_set_) && (value_ == other.value_);
115+
}
116+
117+
/**
118+
* Less than comparisor operator.
119+
*/
120+
bool operator < (
121+
const InstanceHandleValue_t& other) const noexcept
122+
{
123+
if (has_been_set_)
124+
{
125+
return other.has_been_set_ && value_ < other.value_;
126+
}
127+
128+
return other.has_been_set_;
129+
}
130+
131+
private:
132+
133+
//! Hash value
134+
KeyHash_t value_ {};
135+
//! Flag indicating if value_ has been modified since the creation of this object
136+
bool has_been_set_ = false;
137+
};
138+
30139
/**
31140
* Struct InstanceHandle_t, used to contain the key for WITH_KEY topics.
32141
* @ingroup COMMON_MODULE
33142
*/
34143
struct RTPS_DllAPI InstanceHandle_t
35144
{
36145
//!Value
37-
octet value[16];
38-
InstanceHandle_t()
39-
{
40-
for (uint8_t i = 0; i < 16; i++)
41-
{
42-
value[i] = 0;
43-
}
44-
}
146+
InstanceHandleValue_t value;
147+
148+
InstanceHandle_t() noexcept = default;
45149

46150
InstanceHandle_t(
47-
const InstanceHandle_t& ihandle)
48-
{
49-
for (uint8_t i = 0; i < 16; i++)
50-
{
51-
value[i] = ihandle.value[i];
52-
}
53-
}
151+
const InstanceHandle_t& ihandle) noexcept = default;
54152

55153
InstanceHandle_t(
56-
const GUID_t& guid)
154+
const GUID_t& guid) noexcept
57155
{
58-
for (uint8_t i = 0; i < 16; ++i)
59-
{
60-
if (i < 12)
61-
{
62-
value[i] = guid.guidPrefix.value[i];
63-
}
64-
else
65-
{
66-
value[i] = guid.entityId.value[i - 12];
67-
}
68-
}
156+
*this = guid;
69157
}
70158

71159
/**
72160
* Assignment operator
73161
* @param ihandle Instance handle to copy the data from
74162
*/
75163
InstanceHandle_t& operator =(
76-
const InstanceHandle_t& ihandle)
77-
{
78-
79-
for (uint8_t i = 0; i < 16; i++)
80-
{
81-
value[i] = ihandle.value[i];
82-
}
83-
return *this;
84-
}
164+
const InstanceHandle_t& ihandle) noexcept = default;
85165

86166
/**
87167
* Assignment operator
88168
* @param guid GUID to copy the data from
89169
*/
90170
InstanceHandle_t& operator =(
91-
const GUID_t& guid)
171+
const GUID_t& guid) noexcept
92172
{
93-
for (uint8_t i = 0; i < 16; i++)
94-
{
95-
if (i < 12)
96-
{
97-
value[i] = guid.guidPrefix.value[i];
98-
}
99-
else
100-
{
101-
value[i] = guid.entityId.value[i - 12];
102-
}
103-
}
173+
octet* dst = value;
174+
memcpy(dst, guid.guidPrefix.value, 12);
175+
memcpy(&dst[12], guid.entityId.value, 4);
104176
return *this;
105177
}
106178

107179
/**
108180
* Know if the instance handle is defined
109181
* @return True if the values are not zero.
110182
*/
111-
bool isDefined() const
183+
bool isDefined() const noexcept
112184
{
113-
for (uint8_t i = 0; i < 16; ++i)
114-
{
115-
if (value[i] != 0)
116-
{
117-
return true;
118-
}
119-
}
120-
return false;
185+
return value.has_been_set();
121186
}
122187

123188
// TODO Review this conversion once InstanceHandle_t is implemented as DDS standard defines
124-
explicit operator const GUID_t&() const
189+
explicit operator const GUID_t&() const noexcept
125190
{
126191
return *reinterpret_cast<const GUID_t*>(this);
127192
}
@@ -140,21 +205,14 @@ const InstanceHandle_t c_InstanceHandle_Unknown;
140205
*/
141206
inline bool operator ==(
142207
const InstanceHandle_t& ihandle1,
143-
const InstanceHandle_t& ihandle2)
208+
const InstanceHandle_t& ihandle2) noexcept
144209
{
145-
for (uint8_t i = 0; i < 16; ++i)
146-
{
147-
if (ihandle1.value[i] != ihandle2.value[i])
148-
{
149-
return false;
150-
}
151-
}
152-
return true;
210+
return ihandle1.value == ihandle2.value;
153211
}
154212

155213
inline bool operator !=(
156214
const InstanceHandle_t& ihandle1,
157-
const InstanceHandle_t& ihandle2)
215+
const InstanceHandle_t& ihandle2) noexcept
158216
{
159217
return !(ihandle1 == ihandle2);
160218
}
@@ -168,20 +226,11 @@ inline bool operator !=(
168226
*/
169227
inline void iHandle2GUID(
170228
GUID_t& guid,
171-
const InstanceHandle_t& ihandle)
229+
const InstanceHandle_t& ihandle) noexcept
172230
{
173-
for (uint8_t i = 0; i < 16; ++i)
174-
{
175-
if (i < 12)
176-
{
177-
guid.guidPrefix.value[i] = ihandle.value[i];
178-
}
179-
else
180-
{
181-
guid.entityId.value[i - 12] = ihandle.value[i];
182-
}
183-
}
184-
return;
231+
const octet* value = ihandle.value;
232+
memcpy(guid.guidPrefix.value, value, 12);
233+
memcpy(guid.entityId.value, &value[12], 4);
185234
}
186235

187236
/**
@@ -190,28 +239,18 @@ inline void iHandle2GUID(
190239
* @return GUID_t
191240
*/
192241
inline GUID_t iHandle2GUID(
193-
const InstanceHandle_t& ihandle)
242+
const InstanceHandle_t& ihandle) noexcept
194243
{
195244
GUID_t guid;
196-
for (uint8_t i = 0; i < 16; ++i)
197-
{
198-
if (i < 12)
199-
{
200-
guid.guidPrefix.value[i] = ihandle.value[i];
201-
}
202-
else
203-
{
204-
guid.entityId.value[i - 12] = ihandle.value[i];
205-
}
206-
}
245+
iHandle2GUID(guid, ihandle);
207246
return guid;
208247
}
209248

210249
inline bool operator <(
211250
const InstanceHandle_t& h1,
212-
const InstanceHandle_t& h2)
251+
const InstanceHandle_t& h2) noexcept
213252
{
214-
return memcmp(h1.value, h2.value, 16) < 0;
253+
return h1.value < h2.value;
215254
}
216255

217256
#ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC

‎test/unittest/dds/subscriber/DataReaderTests.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ class DataReaderTests : public ::testing::Test
151151
{
152152
FooType data;
153153

154-
data.index(1);
154+
data.index(0);
155155
type_.get_key(&data, &handle_ok_);
156156

157157
data.index(2);
@@ -512,7 +512,7 @@ class DataReaderTests : public ::testing::Test
512512

513513
// Send data
514514
DataType data;
515-
data.index(1);
515+
data.index(0);
516516
EXPECT_EQ(ReturnCode_t::RETCODE_OK, data_writer_->write(&data, HANDLE_NIL));
517517

518518
// Wait for data to arrive and check OK should be returned
@@ -883,7 +883,7 @@ TEST_F(DataReaderTests, return_loan)
883883
EXPECT_EQ(ok_code, reader2->enable());
884884

885885
FooType data;
886-
data.index(1);
886+
data.index(0);
887887

888888
// Send a bunch of samples
889889
for (int32_t i = 0; i < num_samples; ++i)
@@ -1030,7 +1030,7 @@ TEST_F(DataReaderTests, resource_limits)
10301030
create_entities(nullptr, reader_qos, SUBSCRIBER_QOS_DEFAULT, writer_qos);
10311031

10321032
FooType data;
1033-
data.index(1);
1033+
data.index(0);
10341034

10351035
// Send a bunch of samples
10361036
for (int32_t i = 0; i < num_samples; ++i)
@@ -1232,7 +1232,7 @@ TEST_F(DataReaderTests, read_unread)
12321232
create_entities(nullptr, reader_qos, SUBSCRIBER_QOS_DEFAULT, writer_qos);
12331233

12341234
FooType data;
1235-
data.index(1);
1235+
data.index(0);
12361236
data.message()[1] = '\0';
12371237

12381238
// Send a bunch of samples
@@ -1527,7 +1527,7 @@ TEST_F(DataReaderTests, Deserialization_errors)
15271527
create_entities(nullptr, reader_qos, SUBSCRIBER_QOS_DEFAULT, writer_qos);
15281528

15291529
FooType data;
1530-
data.index(1);
1530+
data.index(0);
15311531
data.message()[1] = '\0';
15321532

15331533
// Check deserialization errors without loans
@@ -1780,7 +1780,7 @@ TEST_F(DataReaderTests, check_key_history_wholesomeness_on_unmatch)
17801780
FooType sample;
17811781
std::array<char, 256> msg = {"checking robustness"};
17821782

1783-
sample.index(1);
1783+
sample.index(0);
17841784
sample.message(msg);
17851785

17861786
ASSERT_TRUE(data_writer_->write(&sample));
@@ -2053,7 +2053,7 @@ TEST_F(DataReaderTests, read_samples_with_future_changes)
20532053
std::this_thread::sleep_for(std::chrono::milliseconds(100)); // Wait discovery
20542054

20552055
FooType data;
2056-
data.index(1);
2056+
data.index(0);
20572057
data.message()[0] = '\0';
20582058
data.message()[1] = '\0';
20592059

‎versions.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
Forthcoming
22
-----------
33

4+
* Allow zero-valued InstanceHandle_t (ABI break)
45
*
56

67
Version 2.4.0

0 commit comments

Comments
 (0)