From d393d397b618d46e9e8788c9da1b4dcf4c478f59 Mon Sep 17 00:00:00 2001
From: Alexander Bushnev <Alexander@Bushnev.pro>
Date: Fri, 8 Nov 2024 00:30:41 +0100
Subject: [PATCH] Rework zid conversion

---
 include/zenoh-pico/collections/string.h |  4 +++-
 include/zenoh-pico/utils/config.h       |  1 +
 include/zenoh-pico/utils/uuid.h         | 14 ++++++++++++++
 src/api/api.c                           |  3 +--
 src/collections/string.c                | 11 ++++++++---
 src/net/session.c                       |  7 ++-----
 src/protocol/config.c                   | 13 +++++++++++++
 src/transport/multicast.c               | 15 +++------------
 src/transport/unicast.c                 | 21 +++++----------------
 src/utils/uuid.c                        |  6 ++++++
 tests/z_api_alignment_test.c            | 17 ++++++-----------
 tests/z_client_test.c                   | 14 +++++---------
 tests/z_data_struct_test.c              | 13 +++++++++++++
 13 files changed, 80 insertions(+), 59 deletions(-)

diff --git a/include/zenoh-pico/collections/string.h b/include/zenoh-pico/collections/string.h
index ef07a0227..a54ac68dd 100644
--- a/include/zenoh-pico/collections/string.h
+++ b/include/zenoh-pico/collections/string.h
@@ -94,9 +94,11 @@ void _z_string_clear(_z_string_t *s);
 void _z_string_free(_z_string_t **s);
 void _z_string_reset(_z_string_t *s);
 bool _z_string_equals(const _z_string_t *left, const _z_string_t *right);
-_z_string_t _z_string_convert_bytes(const _z_slice_t *bs);
+_z_string_t _z_string_convert_bytes_le(const _z_slice_t *bs);
 _z_string_t _z_string_preallocate(const size_t len);
 
+char *_z_str_from_string_clone(const _z_string_t *str);
+
 _Z_ELEM_DEFINE(_z_string, _z_string_t, _z_string_len, _z_string_clear, _z_string_copy)
 
 static inline void _z_string_elem_move(void *dst, void *src) { _z_string_move((_z_string_t *)dst, (_z_string_t *)src); }
diff --git a/include/zenoh-pico/utils/config.h b/include/zenoh-pico/utils/config.h
index bab52b317..cf67d4189 100644
--- a/include/zenoh-pico/utils/config.h
+++ b/include/zenoh-pico/utils/config.h
@@ -50,6 +50,7 @@ z_result_t _z_config_init(_z_config_t *ps);
  *   value: The value of the property to add.
  */
 z_result_t _zp_config_insert(_z_config_t *ps, uint8_t key, const char *value);
+z_result_t _zp_config_insert_string(_z_config_t *ps, uint8_t key, const _z_string_t *value);
 
 /**
  * Get the property with the given key from a properties map.
diff --git a/include/zenoh-pico/utils/uuid.h b/include/zenoh-pico/utils/uuid.h
index ea612715d..13631460b 100644
--- a/include/zenoh-pico/utils/uuid.h
+++ b/include/zenoh-pico/utils/uuid.h
@@ -14,6 +14,9 @@
 
 #include <stdint.h>
 
+#include "zenoh-pico/collections/string.h"
+#include "zenoh-pico/protocol/core.h"
+
 #ifndef ZENOH_PICO_UTILS_UUID_H
 #define ZENOH_PICO_UTILS_UUID_H
 
@@ -30,6 +33,17 @@ extern "C" {
  */
 void _z_uuid_to_bytes(uint8_t *bytes, const char *uuid_str);
 
+/**
+ * Converts an Zenoh ID to string.
+ *
+ * Parameters:
+ *   id: Zenoh ID.
+ *
+ * Returns:
+ *   ID string representation
+ */
+_z_string_t _z_id_to_string(const _z_id_t *id);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/api/api.c b/src/api/api.c
index c76a9b32e..3f32fbe11 100644
--- a/src/api/api.c
+++ b/src/api/api.c
@@ -715,8 +715,7 @@ z_result_t z_info_routers_zid(const z_loaned_session_t *zs, z_moved_closure_zid_
 z_id_t z_info_zid(const z_loaned_session_t *zs) { return _Z_RC_IN_VAL(zs)->_local_zid; }
 
 z_result_t z_id_to_string(const z_id_t *id, z_owned_string_t *str) {
-    _z_slice_t buf = _z_slice_alias_buf(id->id, sizeof(id->id));
-    str->_val = _z_string_convert_bytes(&buf);
+    str->_val = _z_id_to_string(id);
     if (!_z_string_check(&str->_val)) {
         return _Z_ERR_SYSTEM_OUT_OF_MEMORY;
     }
diff --git a/src/collections/string.c b/src/collections/string.c
index bf74da4f3..e734ee79a 100644
--- a/src/collections/string.c
+++ b/src/collections/string.c
@@ -113,7 +113,7 @@ bool _z_string_equals(const _z_string_t *left, const _z_string_t *right) {
     return (strncmp(_z_string_data(left), _z_string_data(right), _z_string_len(left)) == 0);
 }
 
-_z_string_t _z_string_convert_bytes(const _z_slice_t *bs) {
+_z_string_t _z_string_convert_bytes_le(const _z_slice_t *bs) {
     _z_string_t s = _z_string_null();
     size_t len = bs->len * (size_t)2;
     char *s_val = (char *)z_malloc((len) * sizeof(char));
@@ -122,9 +122,10 @@ _z_string_t _z_string_convert_bytes(const _z_slice_t *bs) {
     }
 
     const char c[] = "0123456789abcdef";
+    size_t pos = bs->len * 2;
     for (size_t i = 0; i < bs->len; i++) {
-        s_val[i * (size_t)2] = c[(bs->start[i] & (uint8_t)0xF0) >> (uint8_t)4];
-        s_val[(i * (size_t)2) + 1] = c[bs->start[i] & (uint8_t)0x0F];
+        s_val[--pos] = c[bs->start[i] & (uint8_t)0x0F];
+        s_val[--pos] = c[(bs->start[i] & (uint8_t)0xF0) >> (uint8_t)4];
     }
     s._slice = _z_slice_from_buf_custom_deleter((const uint8_t *)s_val, len, _z_delete_context_default());
     return s;
@@ -220,4 +221,8 @@ char *_z_str_n_clone(const char *src, size_t len) {
     return dst;
 }
 
+char *_z_str_from_string_clone(const _z_string_t *str) {
+    return _z_str_n_clone((const char *)str->_slice.start, str->_slice.len);
+}
+
 bool _z_str_eq(const char *left, const char *right) { return strcmp(left, right) == 0; }
diff --git a/src/net/session.c b/src/net/session.c
index 0e8fcac4e..a71cbcc3e 100644
--- a/src/net/session.c
+++ b/src/net/session.c
@@ -166,11 +166,8 @@ _z_config_t *_z_info(const _z_session_t *zn) {
     _z_config_t *ps = (_z_config_t *)z_malloc(sizeof(_z_config_t));
     if (ps != NULL) {
         _z_config_init(ps);
-        _z_slice_t local_zid = _z_slice_alias_buf(zn->_local_zid.id, _z_id_len(zn->_local_zid));
-        // TODO(sasahcmc): is it zero terminated???
-        // rework it!!!
-        _z_string_t s = _z_string_convert_bytes(&local_zid);
-        _zp_config_insert(ps, Z_INFO_PID_KEY, _z_string_data(&s));
+        _z_string_t s = _z_id_to_string(&zn->_local_zid);
+        _zp_config_insert_string(ps, Z_INFO_PID_KEY, &s);
         _z_string_clear(&s);
 
         switch (zn->_tp._type) {
diff --git a/src/protocol/config.c b/src/protocol/config.c
index dcdb4f904..1cfcff9c3 100644
--- a/src/protocol/config.c
+++ b/src/protocol/config.c
@@ -19,6 +19,7 @@
 #include <stdio.h>
 #include <string.h>
 
+#include "zenoh-pico/collections/string.h"
 #include "zenoh-pico/utils/pointers.h"
 
 z_result_t _z_config_init(_z_config_t *ps) {
@@ -37,6 +38,18 @@ z_result_t _zp_config_insert(_z_config_t *ps, uint8_t key, const char *value) {
     return ret;
 }
 
+z_result_t _zp_config_insert_string(_z_config_t *ps, uint8_t key, const _z_string_t *value) {
+    z_result_t ret = _Z_RES_OK;
+    char *str = _z_str_from_string_clone(value);
+    char *res = _z_str_intmap_insert(ps, key, str);
+    if (strcmp(res, str) != 0) {
+        ret = _Z_ERR_CONFIG_FAILED_INSERT;
+    }
+    z_free(str);
+
+    return ret;
+}
+
 char *_z_config_get(const _z_config_t *ps, uint8_t key) { return _z_str_intmap_get(ps, key); }
 
 /*------------------ int-string map ------------------*/
diff --git a/src/transport/multicast.c b/src/transport/multicast.c
index 42059f6dc..6c8db89d0 100644
--- a/src/transport/multicast.c
+++ b/src/transport/multicast.c
@@ -19,15 +19,7 @@
 #include <stdlib.h>
 #include <string.h>
 
-#include "zenoh-pico/link/link.h"
-#include "zenoh-pico/transport/common/lease.h"
-#include "zenoh-pico/transport/common/read.h"
-#include "zenoh-pico/transport/common/tx.h"
-#include "zenoh-pico/transport/multicast/rx.h"
-#include "zenoh-pico/transport/multicast/tx.h"
-#include "zenoh-pico/transport/unicast/rx.h"
-#include "zenoh-pico/transport/utils.h"
-#include "zenoh-pico/utils/logging.h"
+#include "zenoh-pico/utils/uuid.h"
 
 #if Z_FEATURE_MULTICAST_TRANSPORT == 1
 void _zp_multicast_fetch_zid(const _z_transport_t *zt, _z_closure_zid_t *callback) {
@@ -45,9 +37,8 @@ void _zp_multicast_info_session(const _z_transport_t *zt, _z_config_t *ps) {
     _z_transport_peer_entry_list_t *xs = zt->_transport._multicast._peers;
     while (xs != NULL) {
         _z_transport_peer_entry_t *peer = _z_transport_peer_entry_list_head(xs);
-        _z_slice_t remote_zid = _z_slice_alias_buf(peer->_remote_zid.id, _z_id_len(peer->_remote_zid));
-        _z_string_t remote_zid_str = _z_string_convert_bytes(&remote_zid);
-        _zp_config_insert(ps, Z_INFO_PEER_PID_KEY, _z_string_data(&remote_zid_str));
+        _z_string_t remote_zid_str = _z_id_to_string(&peer->_remote_zid);
+        _zp_config_insert_string(ps, Z_INFO_PEER_PID_KEY, &remote_zid_str);
         _z_string_clear(&remote_zid_str);
 
         xs = _z_transport_peer_entry_list_tail(xs);
diff --git a/src/transport/unicast.c b/src/transport/unicast.c
index dca9ae0ea..f194b4ee3 100644
--- a/src/transport/unicast.c
+++ b/src/transport/unicast.c
@@ -16,19 +16,8 @@
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdint.h>
-#include <stdlib.h>
-#include <string.h>
-
-#include "zenoh-pico/link/link.h"
-#include "zenoh-pico/transport/common/rx.h"
-#include "zenoh-pico/transport/common/tx.h"
-#include "zenoh-pico/transport/multicast/rx.h"
-#include "zenoh-pico/transport/unicast/lease.h"
-#include "zenoh-pico/transport/unicast/read.h"
-#include "zenoh-pico/transport/unicast/rx.h"
-#include "zenoh-pico/transport/unicast/tx.h"
-#include "zenoh-pico/transport/utils.h"
-#include "zenoh-pico/utils/logging.h"
+
+#include "zenoh-pico/utils/uuid.h"
 
 #if Z_FEATURE_UNICAST_TRANSPORT == 1
 void _zp_unicast_fetch_zid(const _z_transport_t *zt, _z_closure_zid_t *callback) {
@@ -39,9 +28,9 @@ void _zp_unicast_fetch_zid(const _z_transport_t *zt, _z_closure_zid_t *callback)
 
 void _zp_unicast_info_session(const _z_transport_t *zt, _z_config_t *ps) {
     _z_id_t remote_zid = zt->_transport._unicast._remote_zid;
-    _z_slice_t remote_zid_bytes = _z_slice_alias_buf(remote_zid.id, _z_id_len(remote_zid));
-    _z_string_t remote_zid_str = _z_string_convert_bytes(&remote_zid_bytes);
-    _zp_config_insert(ps, Z_INFO_ROUTER_PID_KEY, _z_string_data(&remote_zid_str));
+
+    _z_string_t remote_zid_str = _z_id_to_string(&remote_zid);
+    _zp_config_insert_string(ps, Z_INFO_ROUTER_PID_KEY, &remote_zid_str);
     _z_string_clear(&remote_zid_str);
 }
 
diff --git a/src/utils/uuid.c b/src/utils/uuid.c
index 7b9984871..1f2d97fe6 100644
--- a/src/utils/uuid.c
+++ b/src/utils/uuid.c
@@ -17,6 +17,7 @@
 #include <stdint.h>
 #include <stdlib.h>
 
+#include "zenoh-pico/api/types.h"
 #include "zenoh-pico/utils/pointers.h"
 
 #define UUID_SIZE 16
@@ -32,3 +33,8 @@ void _z_uuid_to_bytes(uint8_t *bytes, const char *uuid_str) {
         bytes = _z_ptr_u8_offset(bytes, 1);
     }
 }
+
+_z_string_t _z_id_to_string(const z_id_t *id) {
+    _z_slice_t buf = _z_slice_alias_buf(id->id, sizeof(id->id));
+    return _z_string_convert_bytes_le(&buf);
+}
diff --git a/tests/z_api_alignment_test.c b/tests/z_api_alignment_test.c
index a8b48eb3e..abef93a46 100644
--- a/tests/z_api_alignment_test.c
+++ b/tests/z_api_alignment_test.c
@@ -132,11 +132,9 @@ int main(int argc, char **argv) {
     zc_init_logger();
     const char *encoding_expected =
         z_sample_kind(sample) == Z_SAMPLE_KIND_PUT ? "zenoh/bytes" : "zenoh/bytes;test_encoding";
-    z_pu
 #endif
 
-        z_view_keyexpr_t key_demo_example,
-        key_demo_example_a, key_demo_example_starstar;
+    z_view_keyexpr_t key_demo_example, key_demo_example_a, key_demo_example_starstar;
     z_view_keyexpr_from_str(&key_demo_example, "demo/example");
     z_view_keyexpr_from_str(&key_demo_example_a, "demo/example/a");
     z_view_keyexpr_from_str(&key_demo_example_starstar, "demo/example/**");
@@ -199,10 +197,9 @@ int main(int argc, char **argv) {
     assert(z_internal_check(s1));
     z_id_t _ret_zid = z_info_zid(z_loan(s1));
     printf("Session 1 with PID: 0x");
-    for (unsigned long i = 0; i < sizeof(_ret_zid); i++) {
-        printf("%.2X", _ret_zid.id[i]);
-    }
-    printf("\n");
+    z_owned_string_t id_str;
+    z_id_to_string(&_ret_zid, &id_str);
+    printf("%.*s\n", (int)z_string_len(z_loan(id_str)), z_string_data(z_loan(id_str)));
 
     z_owned_closure_zid_t _ret_closure_zid;
     z_closure(&_ret_closure_zid, zid_handler, NULL, NULL);
@@ -243,10 +240,8 @@ int main(int argc, char **argv) {
     assert(z_internal_check(s2));
     _ret_zid = z_info_zid(z_loan(s2));
     printf("Session 2 with PID: 0x");
-    for (unsigned long i = 0; i < sizeof(_ret_zid); i++) {
-        printf("%.2X", _ret_zid.id[i]);
-    }
-    printf("\n");
+    z_id_to_string(&_ret_zid, &id_str);
+    printf("%.*s\n", (int)z_string_len(z_loan(id_str)), z_string_data(z_loan(id_str)));
 
 #ifdef ZENOH_PICO
     zp_start_read_task(z_loan_mut(s2), NULL);
diff --git a/tests/z_client_test.c b/tests/z_client_test.c
index ca1547e20..ca230002c 100644
--- a/tests/z_client_test.c
+++ b/tests/z_client_test.c
@@ -20,6 +20,7 @@
 #include "zenoh-pico.h"
 #include "zenoh-pico/api/types.h"
 #include "zenoh-pico/collections/string.h"
+#include "zenoh-pico/utils/uuid.h"
 
 #undef NDEBUG
 #include <assert.h>
@@ -116,11 +117,6 @@ void data_handler(z_loaned_sample_t *sample, void *arg) {
     free(res);
 }
 
-_z_string_t format_id(const z_id_t *id) {
-    _z_slice_t id_as_bytes = _z_slice_alias_buf(id->id, _z_id_len(*id));
-    return _z_string_convert_bytes(&id_as_bytes);
-}
-
 int main(int argc, char **argv) {
     setvbuf(stdout, NULL, _IOLBF, 1024);
 
@@ -137,8 +133,8 @@ int main(int argc, char **argv) {
 
     z_owned_session_t s1;
     assert(z_open(&s1, z_move(config), NULL) == Z_OK);
-    _z_string_t zid1 = format_id(&(_Z_RC_IN_VAL(z_loan(s1))->_local_zid));
-    printf("Session 1 with PID: %s\n", _z_string_data(&zid1));
+    _z_string_t zid1 = _z_id_to_string(&(_Z_RC_IN_VAL(z_loan(s1))->_local_zid));
+    printf("Session 1 with PID: %*.s\n", (int)_z_string_len(&zid1), _z_string_data(&zid1));
     _z_string_clear(&zid1);
 
     // Start the read session session lease loops
@@ -153,8 +149,8 @@ int main(int argc, char **argv) {
     z_owned_session_t s2;
     assert(z_open(&s2, z_move(config), NULL) == Z_OK);
     assert(z_internal_check(s2));
-    _z_string_t zid2 = format_id(&(_Z_RC_IN_VAL(z_loan(s2))->_local_zid));
-    printf("Session 2 with PID: %s\n", _z_string_data(&zid2));
+    _z_string_t zid2 = _z_id_to_string(&(_Z_RC_IN_VAL(z_loan(s2))->_local_zid));
+    printf("Session 2 with PID: %*.s\n", (int)_z_string_len(&zid2), _z_string_data(&zid2));
     _z_string_clear(&zid2);
 
     // Start the read session session lease loops
diff --git a/tests/z_data_struct_test.c b/tests/z_data_struct_test.c
index 0b7cfa702..e3dff0fa5 100644
--- a/tests/z_data_struct_test.c
+++ b/tests/z_data_struct_test.c
@@ -233,11 +233,24 @@ void z_string_array_test(void) {
     z_string_array_drop(z_string_array_move(&a));
 }
 
+void z_id_to_string_test(void) {
+    z_id_t id;
+    for (uint8_t i = 0; i < sizeof(id.id); i++) {
+        id.id[i] = i;
+    }
+    z_owned_string_t id_str;
+    z_id_to_string(&id, &id_str);
+    assert(z_string_len(z_string_loan(&id_str)) == 32);
+    assert(strncmp("0f0e0d0c0b0a09080706050403020100", z_string_data(z_string_loan(&id_str)),
+                   z_string_len(z_string_loan(&id_str))) == 0);
+}
+
 int main(void) {
     entry_list_test();
     str_vec_list_intmap_test();
     z_slice_custom_delete_test();
     z_string_array_test();
+    z_id_to_string_test();
 
     return 0;
 }