Merge pull request #3 from jean-roland/fix_string

Fix some string rework issues

include/zenoh-pico/link/endpoint.h

@@ -49,7 +49,7 @@ typedef struct {
 _Bool _z_locator_eq(const _z_locator_t *left, const _z_locator_t *right);
 
 void _z_locator_init(_z_locator_t *locator);
-char *_z_locator_to_str(const _z_locator_t *l);
+_z_string_t *_z_locator_to_string(const _z_locator_t *loc);
 int8_t _z_locator_from_str(_z_locator_t *lc, const char *s);
 
 size_t _z_locator_size(_z_locator_t *lc);

include/zenoh-pico/protocol/codec/core.h

@@ -72,6 +72,7 @@ int8_t _z_bytes_decode(_z_bytes_t *bs, _z_zbuf_t *buf);
 int8_t _z_zbuf_read_exact(_z_zbuf_t *zbf, uint8_t *dest, size_t length);
 
 int8_t _z_str_encode(_z_wbuf_t *buf, const char *s);
+int8_t _z_zstr_encode(_z_wbuf_t *wbf, const _z_string_t *s);
 int8_t _z_str_decode(char **str, _z_zbuf_t *buf);
 
 size_t _z_encoding_len(const _z_encoding_t *en);

src/api/api.c

@@ -406,6 +406,14 @@ OWNED_FUNCTIONS_PTR(_z_string_vec_t, string_array, _z_owner_noop_copy, _z_string
 VIEW_FUNCTIONS_PTR(_z_string_vec_t, string_array)
 OWNED_FUNCTIONS_PTR(_z_bytes_t, bytes, _z_bytes_copy, _z_bytes_free)
 
+static _z_bytes_t _z_bytes_from_owned_bytes(z_owned_bytes_t *bytes) {
+    _z_bytes_t b = _z_bytes_empty();
+    if ((bytes != NULL) && (bytes->_val != NULL)) {
+        b = _z_bytes_wrap(bytes->_val->start, bytes->_val->len);
+    }
+    return b;
+}
+
 OWNED_FUNCTIONS_RC(sample)
 // TODO(sashacmc): drop != close
 OWNED_FUNCTIONS_RC(session)
@@ -791,9 +799,7 @@ void z_get_options_default(z_get_options_t *options) {
     options->target = z_query_target_default();
     options->consolidation = z_query_consolidation_default();
     options->encoding = z_encoding_default();
-    // TODO(sashacmc): add cleanup or rework to on stack
-    options->payload = (z_owned_bytes_t *)z_malloc(sizeof(z_owned_bytes_t));
-    z_bytes_null(options->payload);
+    options->payload = NULL;
 #if Z_FEATURE_ATTACHMENT == 1
     options->attachment = z_attachment_null();
 #endif
@@ -827,18 +833,19 @@ int8_t z_get(const z_loaned_session_t *zs, const z_loaned_keyexpr_t *keyexpr, co
             opt.consolidation.mode = Z_CONSOLIDATION_MODE_LATEST;
         }
     }
-    z_loaned_bytes_t *loaned_payload = z_bytes_loan_mut(opt.payload);
-    _z_value_t value = {
-        .payload = loaned_payload ? _z_bytes_wrap(loaned_payload->start, loaned_payload->len) : _z_bytes_empty(),
-        .encoding = opt.encoding};
+    // Set value
+    _z_value_t value = {.payload = _z_bytes_from_owned_bytes(opt.payload), .encoding = opt.encoding};
+
     ret = _z_query(&_Z_RC_IN_VAL(zs), *keyexpr, parameters, opt.target, opt.consolidation.mode, value, callback->call,
                    callback->drop, ctx, opt.timeout_ms
 #if Z_FEATURE_ATTACHMENT == 1
                    ,
                    opt.attachment
 #endif
     );
-    z_bytes_drop(opt.payload);
+    if (opt.payload != NULL) {
+        z_bytes_drop(opt.payload);
+    }
     return ret;
 }
 
@@ -914,12 +921,13 @@ int8_t z_query_reply(const z_loaned_query_t *query, const z_loaned_keyexpr_t *ke
     } else {
         opts = *options;
     }
-    z_loaned_bytes_t *loaned_payload = z_bytes_loan_mut(payload);
-    _z_value_t value = {
-        .payload = loaned_payload ? _z_bytes_wrap(loaned_payload->start, loaned_payload->len) : _z_bytes_empty(),
-        .encoding = {.id = opts.encoding.id, .schema = opts.encoding.schema}};
+    // Set value
+    _z_value_t value = {.payload = _z_bytes_from_owned_bytes(payload), .encoding = opts.encoding};
+
     int8_t ret = _z_send_reply(&query->in->val, *keyexpr, value, Z_SAMPLE_KIND_PUT, opts.attachment);
-    z_bytes_drop(payload);
+    if (payload != NULL) {
+        z_bytes_drop(payload);
+    }
     return ret;
 }
 #endif

src/link/endpoint.c

@@ -246,21 +246,23 @@ void __z_locator_onto_str(char *dst, size_t dst_len, const _z_locator_t *loc) {
 }
 
 /**
- * Converts a :c:type:`_z_locator_t` into its string format.
+ * Converts a :c:type:`_z_locator_t` into its _z_string format.
  *
  * Parameters:
- *   loc: :c:type:`_z_locator_t` to be converted into its string format.
+ *   loc: :c:type:`_z_locator_t` to be converted into its _z_string format.
  *
  * Returns:
- *   The pointer to the stringified :c:type:`_z_locator_t`.
+ *   The pointer to the z_stringified :c:type:`_z_locator_t`.
  */
-char *_z_locator_to_str(const _z_locator_t *l) {
-    size_t len = _z_locator_strlen(l) + (size_t)1;
-    char *dst = (char *)z_malloc(len);
-    if (dst != NULL) {
-        __z_locator_onto_str(dst, len, l);
+_z_string_t *_z_locator_to_string(const _z_locator_t *loc) {
+    _z_string_t *s = (_z_string_t *)z_malloc(sizeof(_z_string_t));
+    s->len = _z_locator_strlen(loc) + (size_t)1;
+    s->val = (char *)z_malloc(s->len);
+    if (s->val == NULL) {
+        return NULL;
     }
-    return dst;
+    __z_locator_onto_str(s->val, s->len, loc);
+    return s;
 }
 
 /*------------------ Endpoint ------------------*/
@@ -417,37 +419,37 @@ int8_t _z_endpoint_from_str(_z_endpoint_t *ep, const char *str) {
 
 char *_z_endpoint_to_str(const _z_endpoint_t *endpoint) {
     char *ret = NULL;
-
-    char *locator = _z_locator_to_str(&endpoint->_locator);
-    if (locator != NULL) {
-        size_t len = 1;  // Start with space for the null-terminator
-        len = len + strlen(locator);
-
-        char *config = _z_endpoint_config_to_str(&endpoint->_config, endpoint->_locator._protocol);
-        if (config != NULL) {
-            len = len + (size_t)1;       // Config separator
-            len = len + strlen(config);  // Config content
-        }
-
-        // Reconstruct the endpoint as a string
-        ret = (char *)z_malloc(len);
-        if (ret != NULL) {
-            ret[0] = '\0';
-            len = len - (size_t)1;
-
-            if (len > (size_t)0) {
-                (void)strncat(ret, locator, len);
-                len = len - strlen(locator);
-            }
-
-            if (config != NULL) {
-                if (len > (size_t)0) {
-                    (void)strncat(ret, config, len);
-                    len = len - strlen(config);
-                }
-            }
+    // Retrieve locator
+    _z_string_t *locator = _z_locator_to_string(&endpoint->_locator);
+    if (locator == NULL) {
+        return NULL;
+    }
+    size_t curr_len = locator->len;
+    // Retrieve config
+    char *config = _z_endpoint_config_to_str(&endpoint->_config, endpoint->_locator._protocol);
+    if (config != NULL) {
+        curr_len += strlen(config) + (size_t)1;  // Content + separator;
+    }
+    // Reconstruct the endpoint as a string
+    ret = (char *)z_malloc(curr_len);
+    if (ret == NULL) {
+        return NULL;
+    }
+    ret[0] = '\0';
+    curr_len -= (size_t)1;
+    // Copy locator
+    if (curr_len > (size_t)0) {
+        (void)strncat(ret, locator->val, curr_len);
+        curr_len -= locator->len;
+    }
+    // Copy config
+    if (config != NULL) {
+        if (curr_len > (size_t)0) {
+            (void)strncat(ret, config, curr_len);
+            curr_len -= strlen(config);
         }
     }
-
+    // Clean up
+    _z_string_clear(locator);
     return ret;
 }

src/protocol/codec.c

@@ -259,6 +259,13 @@ int8_t _z_str_encode(_z_wbuf_t *wbf, const char *s) {
     return _z_wbuf_write_bytes(wbf, (const uint8_t *)s, 0, len);
 }
 
+int8_t _z_zstr_encode(_z_wbuf_t *wbf, const _z_string_t *s) {
+    size_t len = s->len - (size_t)1;
+    _Z_RETURN_IF_ERR(_z_zsize_encode(wbf, len))
+    // Note that this does not put the string terminator on the wire.
+    return _z_wbuf_write_bytes(wbf, (const uint8_t *)s->val, 0, len);
+}
+
 int8_t _z_str_decode(char **str, _z_zbuf_t *zbf) {
     int8_t ret = _Z_RES_OK;
 

src/protocol/codec/message.c

@@ -148,8 +148,8 @@ int8_t _z_locators_encode(_z_wbuf_t *wbf, const _z_locator_array_t *la) {
     _Z_DEBUG("Encoding _LOCATORS");
     _Z_RETURN_IF_ERR(_z_zsize_encode(wbf, la->_len))
     for (size_t i = 0; i < la->_len; i++) {
-        char *s = _z_locator_to_str(&la->_val[i]);
-        _Z_RETURN_IF_ERR(_z_str_encode(wbf, s))
+        _z_string_t *s = _z_locator_to_string(&la->_val[i]);
+        _Z_RETURN_IF_ERR(_z_zstr_encode(wbf, s))
         z_free(s);
     }
 

src/session/scout.c

@@ -86,9 +86,9 @@ _z_hello_list_t *__z_scout_loop(const _z_wbuf_t *wbf, const char *locator, unsig
                                 if (n_loc > 0) {
                                     hello->locators = _z_string_vec_make(n_loc);
                                     for (size_t i = 0; i < n_loc; i++) {
-                                        _z_string_vec_append(&hello->locators,
-                                                             _z_string_make_as_ptr(_z_locator_to_str(
-                                                                 &s_msg._body._hello._locators._val[i])));
+                                        _z_string_vec_append(
+                                            &hello->locators,
+                                            _z_locator_to_string(&s_msg._body._hello._locators._val[i]));
                                     }
                                 } else {
                                     // @TODO: construct the locator departing from the sock address

tests/z_msgcodec_test.c

@@ -331,10 +331,10 @@ void assert_eq_locator_array(const _z_locator_array_t *left, const _z_locator_ar
         const _z_locator_t *l = &left->_val[i];
         const _z_locator_t *r = &right->_val[i];
 
-        char *ls = _z_locator_to_str(l);
-        char *rs = _z_locator_to_str(r);
+        _z_string_t *ls = _z_locator_to_string(l);
+        _z_string_t *rs = _z_locator_to_string(r);
 
-        printf("%s:%s", ls, rs);
+        printf("%s:%s", ls->val, rs->val);
         if (i < left->_len - 1) printf(" ");
 
         z_free(ls);