Skip to content

Commit 31d33de

Browse files
committed
Make _z_iosli_copy_bytes() copy rather data than append and harden _z_iosli_copy().
1 parent b382177 commit 31d33de

File tree

3 files changed

+100
-2
lines changed

3 files changed

+100
-2
lines changed

include/zenoh-pico/protocol/iobuf.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,9 @@ static inline void _z_iosli_read_bytes(_z_iosli_t *ios, uint8_t *dst, size_t off
8282
static inline void _z_iosli_copy_bytes(_z_iosli_t *dst, const _z_iosli_t *src) {
8383
size_t length = _z_iosli_readable(src);
8484
assert(dst->_capacity >= length);
85-
(void)memcpy(dst->_buf + dst->_w_pos, src->_buf + src->_r_pos, length);
86-
dst->_w_pos += length;
85+
(void)memcpy(dst->_buf, src->_buf + src->_r_pos, length);
86+
dst->_r_pos = 0;
87+
dst->_w_pos = length;
8788
}
8889
static inline void _z_iosli_write_bytes(_z_iosli_t *ios, const uint8_t *bs, size_t offset, size_t length) {
8990
assert(_z_iosli_writable(ios) >= length);

src/protocol/iobuf.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,11 @@ void _z_iosli_copy(_z_iosli_t *dst, const _z_iosli_t *src) {
102102
dst->_buf = (uint8_t *)z_malloc(src->_capacity);
103103
if (dst->_buf != NULL) {
104104
(void)memcpy(dst->_buf, src->_buf, src->_capacity);
105+
} else {
106+
dst->_r_pos = 0;
107+
dst->_w_pos = 0;
108+
dst->_capacity = 0;
109+
dst->_is_alloc = false;
105110
}
106111
} else {
107112
dst->_buf = src->_buf;

tests/z_iobuf_test.c

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,95 @@ void test_wbuf_wrap_bytes(void) {
609609
printf("Ok\n");
610610
}
611611

612+
static void fill_pattern(uint8_t *buf, size_t n) {
613+
for (size_t i = 0; i < n; i++) {
614+
buf[i] = (uint8_t)(0xA0u + (uint8_t)i);
615+
}
616+
}
617+
618+
void test_copy_bytes_readable_region(void) {
619+
_z_iosli_t src = _z_iosli_make(16);
620+
_z_iosli_t dst = _z_iosli_make(16);
621+
assert(src._buf);
622+
assert(dst._buf);
623+
624+
fill_pattern(src._buf, src._capacity);
625+
626+
// Make readable window be [r_pos..w_pos) = [5..12) => length 7
627+
src._r_pos = 5;
628+
src._w_pos = 12;
629+
630+
// Dirty dst positions to ensure function overwrites them
631+
dst._r_pos = 3;
632+
dst._w_pos = 9;
633+
memset(dst._buf, 0x00, dst._capacity);
634+
635+
_z_iosli_copy_bytes(&dst, &src);
636+
637+
// dst now contains exactly the readable bytes starting at offset 0
638+
assert(dst._r_pos == 0);
639+
assert(dst._w_pos == 7);
640+
assert(memcmp(dst._buf, src._buf + src._r_pos, 7) == 0);
641+
642+
_z_iosli_clear(&src);
643+
_z_iosli_clear(&dst);
644+
}
645+
646+
void test_copy_bytes_exact_capacity_fit(void) {
647+
_z_iosli_t src = _z_iosli_make(32);
648+
assert(src._buf);
649+
650+
// Create a readable region length = 10
651+
src._r_pos = 2;
652+
src._w_pos = 12; // len = 10
653+
fill_pattern(src._buf, src._capacity);
654+
655+
_z_iosli_t dst = _z_iosli_make(10);
656+
assert(dst._buf);
657+
memset(dst._buf, 0xEE, dst._capacity);
658+
659+
// Should succeed because dst capacity == readable length
660+
_z_iosli_copy_bytes(&dst, &src);
661+
662+
assert(dst._r_pos == 0);
663+
assert(dst._w_pos == 10);
664+
assert(memcmp(dst._buf, src._buf + src._r_pos, 10) == 0);
665+
666+
_z_iosli_clear(&src);
667+
_z_iosli_clear(&dst);
668+
}
669+
670+
void test_copy_bytes_does_not_alias(void) {
671+
_z_iosli_t src = _z_iosli_make(16);
672+
_z_iosli_t dst = _z_iosli_make(16);
673+
assert(src._buf && dst._buf);
674+
675+
fill_pattern(src._buf, src._capacity);
676+
src._r_pos = 4;
677+
src._w_pos = 9; // len = 5
678+
679+
_z_iosli_copy_bytes(&dst, &src);
680+
681+
assert(dst._r_pos == 0);
682+
assert(dst._w_pos == 5);
683+
assert(memcmp(dst._buf, src._buf + src._r_pos, 5) == 0);
684+
685+
// Mutate src readable region; dst should remain unchanged (deep copy)
686+
src._buf[src._r_pos + 0] ^= 0xFF;
687+
src._buf[src._r_pos + 1] ^= 0xFF;
688+
689+
// Should differ
690+
assert(memcmp(dst._buf, src._buf + src._r_pos, 5) != 0);
691+
692+
// But dst still equals original pattern:
693+
uint8_t expected_full[16];
694+
fill_pattern(expected_full, sizeof(expected_full));
695+
assert(memcmp(dst._buf, expected_full + 4, 5) == 0);
696+
697+
_z_iosli_clear(&src);
698+
_z_iosli_clear(&dst);
699+
}
700+
612701
/*=============================*/
613702
/* Main */
614703
/*=============================*/
@@ -634,4 +723,7 @@ int main(void) {
634723
wbuf_reusable_write_zbuf_read();
635724
}
636725
test_wbuf_wrap_bytes();
726+
test_copy_bytes_readable_region();
727+
test_copy_bytes_exact_capacity_fit();
728+
test_copy_bytes_does_not_alias();
637729
}

0 commit comments

Comments
 (0)