Skip to content

Commit 698b7b2

Browse files
committed
Refs #23753. Apply suggestions
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
1 parent 10bfb67 commit 698b7b2

2 files changed

Lines changed: 51 additions & 5 deletions

File tree

fastdds_python/src/swig/fastdds/rtps/common/InstanceHandle.i

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ long hash(const eprosima::fastdds::rtps::InstanceHandle_t& handle)
4343

4444
%extend eprosima::fastdds::rtps::InstanceHandleValue_t {
4545

46-
// Constructor from sequence of 16 bytes (tupla/lista/bytes/bytearray)
46+
// Constructor from a sequence of 16 bytes (tuple/list/bytes/bytearray)
4747
InstanceHandleValue_t(PyObject* seq) {
4848
eprosima::fastdds::rtps::InstanceHandleValue_t* self = new eprosima::fastdds::rtps::InstanceHandleValue_t();
4949
SWIG_PYTHON_THREAD_BEGIN_BLOCK;
@@ -85,7 +85,7 @@ long hash(const eprosima::fastdds::rtps::InstanceHandle_t& handle)
8585
if (it)
8686
{
8787
PyObject* item {nullptr};
88-
while ((item = PyIter_Next(it)))
88+
while ((item = PyIter_Next(it)) && count < 16)
8989
{
9090
long val = PyLong_AsLong(item);
9191
Py_DECREF(item);
@@ -108,7 +108,7 @@ long hash(const eprosima::fastdds::rtps::InstanceHandle_t& handle)
108108
++count;
109109
}
110110
Py_DECREF(it);
111-
if (count != 16)
111+
if ((nullptr != item || count != 16) && nullptr != self)
112112
{
113113
delete self;
114114
self = nullptr;
@@ -171,12 +171,12 @@ long hash(const eprosima::fastdds::rtps::InstanceHandle_t& handle)
171171

172172
// Setter from sequence (tuple/list/bytes/bytearray)
173173
void from_sequence(PyObject* seq) {
174-
// Reutiliza el constructor para validar y copiar
174+
// Reuse the constructor to validate and copy
175175
eprosima::fastdds::rtps::InstanceHandleValue_t* tmp = new_eprosima_fastdds_rtps_InstanceHandleValue_t(seq);
176176
if (nullptr != tmp)
177177
{
178178
for (int i = 0; i < 16; ++i) $self->value[i] = (*tmp)[i];
179-
delete tmp; // evitar fuga
179+
delete tmp; // avoid memory leak
180180
}
181181
}
182182

fastdds_python/test/unittest/dds/common/test_InstanceHandle.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,49 @@ def test_create_instance_handle_from_list_with_more_elements():
9393
ih.value = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18]
9494
repr = exception.getrepr()
9595
assert str(repr).split("\n")[0] == "ValueError: Expected 16 elements"
96+
97+
98+
def test_create_instance_handle_from_bytearray_with_with_negative_number():
99+
ih = fastdds.InstanceHandle_t()
100+
with pytest.raises(ValueError) as exception:
101+
ih.value = bytearray([1, 2, 3, 4, 5, 6, 7, 8, 9, -10, 11, 12, 13, 14, 15, 16])
102+
assert str(exception.value) == "byte must be in range(0, 256)"
103+
104+
105+
def test_create_instance_handle_from_bytearray_with_large_number():
106+
ih = fastdds.InstanceHandle_t()
107+
with pytest.raises(ValueError) as exception:
108+
ih.value = bytearray([1, 2, 3, 4, 5, 6, 7, 8, 9, 1000, 11, 12, 13, 14, 15, 16])
109+
assert str(exception.value) == "byte must be in range(0, 256)"
110+
111+
112+
def test_create_instance_handle_from_tuple_with_negative_number():
113+
ih = fastdds.InstanceHandle_t()
114+
with pytest.raises(SystemError) as exception:
115+
ih.value = (1, 2, 3, 4, 5, 6, 7, 8, 9, -10, 11, 12, 13, 14, -15, 16)
116+
repr = exception.getrepr()
117+
assert str(repr).split("\n")[0] == "ValueError: Each value must be in 0..255"
118+
119+
120+
def test_create_instance_handle_from_tuple_with_large_number():
121+
ih = fastdds.InstanceHandle_t()
122+
with pytest.raises(SystemError) as exception:
123+
ih.value = (1, 2, 3, 4, 5, 6, 7, 8, 9, 1000, 11, 12, 13, 14, 15, 16)
124+
repr = exception.getrepr()
125+
assert str(repr).split("\n")[0] == "ValueError: Each value must be in 0..255"
126+
127+
128+
def test_create_instance_handle_from_list_with_negative_number():
129+
ih = fastdds.InstanceHandle_t()
130+
with pytest.raises(SystemError) as exception:
131+
ih.value = [1, 2, 3, 4, 5, 6, 7, 8, 9, -10, 11, 12, 13, 14, 15, 16]
132+
repr = exception.getrepr()
133+
assert str(repr).split("\n")[0] == "ValueError: Each value must be in 0..255"
134+
135+
136+
def test_create_instance_handle_from_list_with_large_number():
137+
ih = fastdds.InstanceHandle_t()
138+
with pytest.raises(SystemError) as exception:
139+
ih.value = [1, 2, 3, 4, 5, 6, 7, 8, 9, 1000, 11, 12, 13, 14, 15, 16]
140+
repr = exception.getrepr()
141+
assert str(repr).split("\n")[0] == "ValueError: Each value must be in 0..255"

0 commit comments

Comments
 (0)