Skip to content

Commit

Permalink
fix undeclaration error reporting
Browse files Browse the repository at this point in the history
  • Loading branch information
p-avital committed Aug 25, 2023
1 parent 82f060f commit a46e236
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 84 deletions.
3 changes: 2 additions & 1 deletion src/api/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,8 @@ z_owned_subscriber_t z_declare_subscriber(z_session_t zs, z_keyexpr_t keyexpr, z
_z_resource_t *r = _z_get_resource_by_key(zs._val, &keyexpr);
if (r == NULL) {
char *wild = strpbrk(keyexpr._suffix, "*$");
if (wild != NULL) {
if (wild != NULL && wild != keyexpr._suffix) {
wild -= 1;
size_t len = wild - keyexpr._suffix;
char *suffix = z_malloc(len + 1);
memcpy(suffix, keyexpr._suffix, len);
Expand Down
12 changes: 7 additions & 5 deletions src/net/primitives.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
#include "zenoh-pico/session/resource.h"
#include "zenoh-pico/session/subscription.h"
#include "zenoh-pico/session/utils.h"
#include "zenoh-pico/utils/logging.h"
#include "zenoh-pico/utils/result.h"

/*------------------ Scouting ------------------*/
void _z_scout(const z_what_t what, const _z_id_t zid, const char *locator, const uint32_t timeout,
Expand Down Expand Up @@ -74,8 +76,8 @@ uint16_t _z_declare_resource(_z_session_t *zn, _z_keyexpr_t keyexpr) {

int8_t _z_undeclare_resource(_z_session_t *zn, uint16_t rid) {
int8_t ret = _Z_RES_OK;

_z_resource_t *r = _z_get_resource_by_id(zn, _Z_RESOURCE_IS_LOCAL, rid);
_Z_DEBUG("Undeclaring local keyexpr %d\n", rid);
_z_resource_t *r = _z_get_resource_by_id(zn, _Z_KEYEXPR_MAPPING_LOCAL, rid);
if (r != NULL) {
// Build the declare message to send on the wire
_z_declaration_t declaration = _z_make_undecl_keyexpr(rid);
Expand Down Expand Up @@ -113,7 +115,7 @@ _z_publisher_t *_z_declare_publisher(_z_session_t *zn, _z_keyexpr_t keyexpr, z_c
}

int8_t _z_undeclare_publisher(_z_publisher_t *pub) {
int8_t ret = _Z_ERR_GENERIC;
int8_t ret = _Z_RES_OK;

if (pub != NULL) {
// Build the declare message to send on the wire
Expand Down Expand Up @@ -232,7 +234,7 @@ _z_queryable_t *_z_declare_queryable(_z_session_t *zn, _z_keyexpr_t keyexpr, _Bo
}

int8_t _z_undeclare_queryable(_z_queryable_t *qle) {
int8_t ret = _Z_ERR_GENERIC;
int8_t ret = _Z_RES_OK;

if (qle != NULL) {
_z_questionable_sptr_t *q = _z_get_questionable_by_id(qle->_zn, qle->_id);
Expand All @@ -244,7 +246,7 @@ int8_t _z_undeclare_queryable(_z_queryable_t *qle) {
// Only if message is successfully send, local queryable state can be removed
_z_unregister_questionable(qle->_zn, q);
} else {
ret = _Z_ERR_ENTITY_UNKNOWN;
ret = _Z_ERR_TRANSPORT_TX_FAILED;
}
_z_n_msg_clear(&n_msg);

Expand Down
62 changes: 34 additions & 28 deletions src/session/resource.c
Original file line number Diff line number Diff line change
Expand Up @@ -228,24 +228,29 @@ int16_t _z_register_resource(_z_session_t *zn, _z_keyexpr_t key, uint16_t id, ui
#endif // Z_MULTI_THREAD == 1

if (key._id != Z_RESOURCE_ID_NONE) {
if (parent_mapping == mapping || parent_mapping == _Z_KEYEXPR_MAPPING_LOCAL) {
_z_resource_t *parent = __unsafe_z_get_resource_by_id(zn, mapping, key._id);
if (parent_mapping == mapping) {
_z_resource_t *parent = __unsafe_z_get_resource_by_id(zn, parent_mapping, key._id);
parent->_refcount++;
} else {
key = __unsafe_z_get_expanded_key_from_key(zn, &key);
}
}
_z_resource_t *res = z_malloc(sizeof(_z_resource_t));
if ((key._suffix != NULL || mapping != _Z_KEYEXPR_MAPPING_LOCAL) && res != NULL) {
res->_refcount = 1;
res->_key = _z_keyexpr_to_owned(key);
ret = id == Z_RESOURCE_ID_NONE ? _z_get_resource_id(zn) : id;
res->_id = ret;
// Register the resource
if (mapping == _Z_KEYEXPR_MAPPING_LOCAL) {
zn->_local_resources = _z_resource_list_push(zn->_local_resources, res);
ret = key._id;
if ((key._suffix != NULL)) {
_z_resource_t *res = z_malloc(sizeof(_z_resource_t));
if (res == NULL) {
ret = Z_RESOURCE_ID_NONE;
} else {
zn->_remote_resources = _z_resource_list_push(zn->_remote_resources, res);
res->_refcount = 1;
res->_key = _z_keyexpr_to_owned(key);
ret = id == Z_RESOURCE_ID_NONE ? _z_get_resource_id(zn) : id;
res->_id = ret;
// Register the resource
if (mapping == _Z_KEYEXPR_MAPPING_LOCAL) {
zn->_local_resources = _z_resource_list_push(zn->_local_resources, res);
} else {
zn->_remote_resources = _z_resource_list_push(zn->_remote_resources, res);
}
}
} else {
ret = Z_RESOURCE_ID_NONE;
Expand All @@ -260,18 +265,21 @@ int16_t _z_register_resource(_z_session_t *zn, _z_keyexpr_t key, uint16_t id, ui

void _z_unregister_resource(_z_session_t *zn, uint16_t id, uint16_t mapping) {
_Bool is_local = mapping == _Z_KEYEXPR_MAPPING_LOCAL;
_Z_DEBUG("unregistering: id %d, mapping: %d\n", id, mapping);
#if Z_MULTI_THREAD == 1
_z_mutex_lock(&zn->_mutex_inner);
#endif // Z_MULTI_THREAD == 1
_z_resource_list_t **parent_mut = is_local ? &zn->_local_resources : &zn->_remote_resources;
while (id != 0) {
_z_resource_list_t **list = is_local ? &zn->_local_resources : &zn->_remote_resources;
_z_resource_list_t *tail = *list;
while (list) {
_z_resource_t *head = _z_resource_list_head(tail);
_z_resource_list_t *parent = *parent_mut;
while (parent != NULL) {
_z_resource_t *head = _z_resource_list_head(parent);
_Z_DEBUG("unreg check: id %d, mapping: %d, refcount: %d\n", head->_id, _z_keyexpr_mapping_id(&head->_key),
head->_refcount);
if (head && head->_id == id && _z_keyexpr_mapping_id(&head->_key) == mapping) {
head->_refcount--;
if (head->_refcount == 0) {
*list = _z_resource_list_pop(*list, &head);
*parent_mut = _z_resource_list_pop(parent, &head);
id = head->_key._id;
mapping = _z_keyexpr_mapping_id(&head->_key);
_z_resource_free(&head);
Expand All @@ -280,28 +288,26 @@ void _z_unregister_resource(_z_session_t *zn, uint16_t id, uint16_t mapping) {
}
break;
}
tail = _z_resource_list_tail(tail);
parent_mut = &parent->_tail;
parent = *parent_mut;
}
}
#if Z_MULTI_THREAD == 1
_z_mutex_unlock(&zn->_mutex_inner);
#endif // Z_MULTI_THREAD == 1
}

_Bool _z_unregister_resource_for_peer_filter(const _z_resource_t *candidate, const _z_resource_t *ctx) {
uint16_t mapping = ctx->_id;
return _z_keyexpr_mapping_id(&candidate->_key) == mapping;
}
void _z_unregister_resources_for_peer(_z_session_t *zn, uint16_t mapping) {
#if Z_MULTI_THREAD == 1
_z_mutex_lock(&zn->_mutex_inner);
#endif // Z_MULTI_THREAD == 1
_z_resource_list_t *list = zn->_remote_resources;
while (list) {
_z_resource_t *head = _z_resource_list_head(list);
if (head && _z_keyexpr_mapping_id(&head->_key) == mapping) {
_z_resource_list_pop(list, &head);
_z_resource_free(&head);
} else {
list = _z_resource_list_tail(list);
}
}
_z_resource_t ctx = {._id = mapping, ._refcount = 0, ._key = {0}};
zn->_remote_resources =
_z_resource_list_drop_filter(zn->_remote_resources, _z_unregister_resource_for_peer_filter, &ctx);

#if Z_MULTI_THREAD == 1
_z_mutex_unlock(&zn->_mutex_inner);
Expand Down
2 changes: 1 addition & 1 deletion tests/api.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#

TESTBIN="$1"
TESTDIR=$(dirname "$0")
TESTDIR=$(realpath $(dirname "$0"))

if [ "$OSTYPE" = "msys" ]; then
TESTBIN="$TESTDIR/Debug/$TESTBIN.exe"
Expand Down
2 changes: 1 addition & 1 deletion tests/multicast.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#

TESTBIN="$1"
TESTDIR=$(dirname "$0")
TESTDIR=$(realpath $(dirname "$0"))

if [ "$OSTYPE" = "msys" ]; then
TESTBIN="$TESTDIR/Debug/$TESTBIN.exe"
Expand Down
2 changes: 1 addition & 1 deletion tests/routed.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#

TESTBIN="$1"
TESTDIR=$(dirname "$0")
TESTDIR=$(realpath $(dirname "$0"))

if [ "$OSTYPE" = "msys" ]; then
TESTBIN="$TESTDIR/Debug/$TESTBIN.exe"
Expand Down
Loading

0 comments on commit a46e236

Please sign in to comment.