From 7d7f5e3e22d4a07e37e90f73a61193fc2a664b39 Mon Sep 17 00:00:00 2001 From: Jiri Svoboda Date: Sun, 17 Sep 2023 11:56:59 +0200 Subject: [PATCH] loc_server_register() should be callable more than once (implementation) We create a new session for each loc_server_register() / loc_srv_t object. Alternatively we could multiplex all to a single connection and then demultiplex them in the server, but this seemed simpler at the moment. We add a test case to libc test suite. --- uspace/lib/c/generic/loc.c | 177 +++++++++++-------------------- uspace/lib/c/include/loc.h | 4 +- uspace/lib/c/include/types/loc.h | 4 +- uspace/lib/c/meson.build | 2 + uspace/lib/c/test/loc.c | 85 +++++++++++++++ uspace/lib/c/test/main.c | 2 + 6 files changed, 156 insertions(+), 118 deletions(-) create mode 100644 uspace/lib/c/test/loc.c diff --git a/uspace/lib/c/generic/loc.c b/uspace/lib/c/generic/loc.c index 981f802a43..9244ca77d0 100644 --- a/uspace/lib/c/generic/loc.c +++ b/uspace/lib/c/generic/loc.c @@ -38,10 +38,7 @@ #include #include -static FIBRIL_MUTEX_INITIALIZE(loc_supp_block_mutex); static FIBRIL_MUTEX_INITIALIZE(loc_cons_block_mutex); - -static FIBRIL_MUTEX_INITIALIZE(loc_supplier_mutex); static FIBRIL_MUTEX_INITIALIZE(loc_consumer_mutex); static FIBRIL_MUTEX_INITIALIZE(loc_callback_mutex); @@ -49,10 +46,7 @@ static bool loc_callback_created = false; static loc_cat_change_cb_t cat_change_cb = NULL; static void *cat_change_arg = NULL; -static async_sess_t *loc_supp_block_sess = NULL; static async_sess_t *loc_cons_block_sess = NULL; - -static async_sess_t *loc_supplier_sess = NULL; static async_sess_t *loc_consumer_sess = NULL; static void loc_cb_conn(ipc_call_t *icall, void *arg) @@ -107,7 +101,7 @@ static errno_t loc_callback_create(void) { if (!loc_callback_created) { async_exch_t *exch = - loc_exchange_begin_blocking(INTERFACE_LOC_CONSUMER); + loc_exchange_begin_blocking(); ipc_call_t answer; aid_t req = async_send_0(exch, LOC_CALLBACK_CREATE, &answer); @@ -133,99 +127,52 @@ static errno_t loc_callback_create(void) } /** Start an async exchange on the loc session (blocking). - * - * @param iface Location service interface to choose * * @return New exchange. * */ -async_exch_t *loc_exchange_begin_blocking(iface_t iface) +async_exch_t *loc_exchange_begin_blocking(void) { - switch (iface) { - case INTERFACE_LOC_SUPPLIER: - fibril_mutex_lock(&loc_supp_block_mutex); - - while (loc_supp_block_sess == NULL) { - clone_session(&loc_supplier_mutex, loc_supplier_sess, - &loc_supp_block_sess); - - if (loc_supp_block_sess == NULL) - loc_supp_block_sess = - service_connect_blocking(SERVICE_LOC, - INTERFACE_LOC_SUPPLIER, 0, NULL); - } + fibril_mutex_lock(&loc_cons_block_mutex); - fibril_mutex_unlock(&loc_supp_block_mutex); + while (loc_cons_block_sess == NULL) { + clone_session(&loc_consumer_mutex, loc_consumer_sess, + &loc_cons_block_sess); - clone_session(&loc_supplier_mutex, loc_supp_block_sess, - &loc_supplier_sess); - - return async_exchange_begin(loc_supp_block_sess); - case INTERFACE_LOC_CONSUMER: - fibril_mutex_lock(&loc_cons_block_mutex); - - while (loc_cons_block_sess == NULL) { - clone_session(&loc_consumer_mutex, loc_consumer_sess, - &loc_cons_block_sess); - - if (loc_cons_block_sess == NULL) - loc_cons_block_sess = - service_connect_blocking(SERVICE_LOC, - INTERFACE_LOC_CONSUMER, 0, NULL); - } + if (loc_cons_block_sess == NULL) + loc_cons_block_sess = + service_connect_blocking(SERVICE_LOC, + INTERFACE_LOC_CONSUMER, 0, NULL); + } - fibril_mutex_unlock(&loc_cons_block_mutex); + fibril_mutex_unlock(&loc_cons_block_mutex); - clone_session(&loc_consumer_mutex, loc_cons_block_sess, - &loc_consumer_sess); + clone_session(&loc_consumer_mutex, loc_cons_block_sess, + &loc_consumer_sess); - return async_exchange_begin(loc_cons_block_sess); - default: - return NULL; - } + return async_exchange_begin(loc_cons_block_sess); } /** Start an async exchange on the loc session. - * - * @param iface Location service interface to choose * * @return New exchange. * */ -async_exch_t *loc_exchange_begin(iface_t iface) +async_exch_t *loc_exchange_begin(void) { - switch (iface) { - case INTERFACE_LOC_SUPPLIER: - fibril_mutex_lock(&loc_supplier_mutex); - - if (loc_supplier_sess == NULL) - loc_supplier_sess = - service_connect(SERVICE_LOC, - INTERFACE_LOC_SUPPLIER, 0, NULL); - - fibril_mutex_unlock(&loc_supplier_mutex); - - if (loc_supplier_sess == NULL) - return NULL; - - return async_exchange_begin(loc_supplier_sess); - case INTERFACE_LOC_CONSUMER: - fibril_mutex_lock(&loc_consumer_mutex); - - if (loc_consumer_sess == NULL) - loc_consumer_sess = - service_connect(SERVICE_LOC, - INTERFACE_LOC_CONSUMER, 0, NULL); + fibril_mutex_lock(&loc_consumer_mutex); - fibril_mutex_unlock(&loc_consumer_mutex); + if (loc_consumer_sess == NULL) + loc_consumer_sess = + service_connect(SERVICE_LOC, + INTERFACE_LOC_CONSUMER, 0, NULL); - if (loc_consumer_sess == NULL) - return NULL; + fibril_mutex_unlock(&loc_consumer_mutex); - return async_exchange_begin(loc_consumer_sess); - default: + if (loc_consumer_sess == NULL) return NULL; - } + + return async_exchange_begin(loc_consumer_sess); } /** Finish an async exchange on the loc session. @@ -239,9 +186,6 @@ void loc_exchange_end(async_exch_t *exch) } /** Register new server with loc. - * - * XXX Proper impementation - currently cannot actually call - * this function more than once. * * @param name Server name * @param rsrv Place to store new server object on success @@ -256,7 +200,14 @@ errno_t loc_server_register(const char *name, loc_srv_t **rsrv) if (srv == NULL) return ENOMEM; - exch = loc_exchange_begin_blocking(INTERFACE_LOC_SUPPLIER); + srv->sess = service_connect_blocking(SERVICE_LOC, + INTERFACE_LOC_SUPPLIER, 0, NULL); + if (srv->sess == NULL) { + free(srv); + return ENOMEM; + } + + exch = async_exchange_begin(srv->sess); ipc_call_t answer; aid_t req = async_send_2(exch, LOC_SERVER_REGISTER, 0, 0, &answer); @@ -264,7 +215,8 @@ errno_t loc_server_register(const char *name, loc_srv_t **rsrv) if (retval != EOK) { async_forget(req); - loc_exchange_end(exch); + async_exchange_end(exch); + async_hangup(srv->sess); free(srv); return retval; } @@ -277,9 +229,10 @@ errno_t loc_server_register(const char *name, loc_srv_t **rsrv) * certain circumstances. */ async_wait_for(req, &retval); - loc_exchange_end(exch); + async_exchange_end(exch); if (retval != EOK) { + async_hangup(srv->sess); free(srv); return retval; } @@ -292,12 +245,11 @@ errno_t loc_server_register(const char *name, loc_srv_t **rsrv) * * Unregister server and free server object. * - * XXX Proper implementation - * * @param srv Server object */ void loc_server_unregister(loc_srv_t *srv) { + async_hangup(srv->sess); free(srv); } @@ -311,17 +263,14 @@ void loc_server_unregister(loc_srv_t *srv) errno_t loc_service_register(loc_srv_t *srv, const char *fqsn, service_id_t *sid) { - async_exch_t *exch = loc_exchange_begin_blocking(INTERFACE_LOC_SUPPLIER); - - (void)srv; - + async_exch_t *exch = async_exchange_begin(srv->sess); ipc_call_t answer; aid_t req = async_send_0(exch, LOC_SERVICE_REGISTER, &answer); errno_t retval = async_data_write_start(exch, fqsn, str_size(fqsn)); if (retval != EOK) { async_forget(req); - loc_exchange_end(exch); + async_exchange_end(exch); return retval; } @@ -331,7 +280,7 @@ errno_t loc_service_register(loc_srv_t *srv, const char *fqsn, * certain circumstances. */ async_wait_for(req, &retval); - loc_exchange_end(exch); + async_exchange_end(exch); if (retval != EOK) { if (sid != NULL) @@ -356,11 +305,9 @@ errno_t loc_service_unregister(loc_srv_t *srv, service_id_t sid) async_exch_t *exch; errno_t retval; - (void)srv; - - exch = loc_exchange_begin_blocking(INTERFACE_LOC_SUPPLIER); + exch = async_exchange_begin(srv->sess); retval = async_req_1_0(exch, LOC_SERVICE_UNREGISTER, sid); - loc_exchange_end(exch); + async_exchange_end(exch); return (errno_t)retval; } @@ -371,9 +318,9 @@ errno_t loc_service_get_id(const char *fqdn, service_id_t *handle, async_exch_t *exch; if (flags & IPC_FLAG_BLOCKING) - exch = loc_exchange_begin_blocking(INTERFACE_LOC_CONSUMER); + exch = loc_exchange_begin_blocking(); else { - exch = loc_exchange_begin(INTERFACE_LOC_CONSUMER); + exch = loc_exchange_begin(); if (exch == NULL) return errno; } @@ -424,7 +371,7 @@ static errno_t loc_get_name_internal(sysarg_t method, sysarg_t id, char **name) errno_t dretval; *name = NULL; - exch = loc_exchange_begin_blocking(INTERFACE_LOC_CONSUMER); + exch = loc_exchange_begin_blocking(); ipc_call_t answer; aid_t req = async_send_1(exch, method, id, &answer); @@ -504,9 +451,9 @@ errno_t loc_namespace_get_id(const char *name, service_id_t *handle, async_exch_t *exch; if (flags & IPC_FLAG_BLOCKING) - exch = loc_exchange_begin_blocking(INTERFACE_LOC_CONSUMER); + exch = loc_exchange_begin_blocking(); else { - exch = loc_exchange_begin(INTERFACE_LOC_CONSUMER); + exch = loc_exchange_begin(); if (exch == NULL) return errno; } @@ -553,9 +500,9 @@ errno_t loc_category_get_id(const char *name, category_id_t *cat_id, async_exch_t *exch; if (flags & IPC_FLAG_BLOCKING) - exch = loc_exchange_begin_blocking(INTERFACE_LOC_CONSUMER); + exch = loc_exchange_begin_blocking(); else { - exch = loc_exchange_begin(INTERFACE_LOC_CONSUMER); + exch = loc_exchange_begin(); if (exch == NULL) return errno; } @@ -589,7 +536,7 @@ errno_t loc_category_get_id(const char *name, category_id_t *cat_id, loc_object_type_t loc_id_probe(service_id_t handle) { - async_exch_t *exch = loc_exchange_begin_blocking(INTERFACE_LOC_CONSUMER); + async_exch_t *exch = loc_exchange_begin_blocking(); sysarg_t type; errno_t retval = async_req_1_1(exch, LOC_ID_PROBE, handle, &type); @@ -620,7 +567,7 @@ async_sess_t *loc_service_connect(service_id_t handle, iface_t iface, */ int loc_null_create(void) { - async_exch_t *exch = loc_exchange_begin_blocking(INTERFACE_LOC_CONSUMER); + async_exch_t *exch = loc_exchange_begin_blocking(); sysarg_t null_id; errno_t retval = async_req_0_1(exch, LOC_NULL_CREATE, &null_id); @@ -635,7 +582,7 @@ int loc_null_create(void) void loc_null_destroy(int null_id) { - async_exch_t *exch = loc_exchange_begin_blocking(INTERFACE_LOC_CONSUMER); + async_exch_t *exch = loc_exchange_begin_blocking(); async_req_1_0(exch, LOC_NULL_DESTROY, (sysarg_t) null_id); loc_exchange_end(exch); } @@ -664,9 +611,9 @@ errno_t loc_service_add_to_cat(loc_srv_t *srv, service_id_t svc_id, async_exch_t *exch; errno_t retval; - exch = loc_exchange_begin_blocking(INTERFACE_LOC_SUPPLIER); + exch = async_exchange_begin(srv->sess); retval = async_req_2_0(exch, LOC_SERVICE_ADD_TO_CAT, svc_id, cat_id); - loc_exchange_end(exch); + async_exchange_end(exch); return retval; } @@ -685,7 +632,7 @@ static size_t loc_count_services_internal(async_exch_t *exch, size_t loc_count_namespaces(void) { - async_exch_t *exch = loc_exchange_begin_blocking(INTERFACE_LOC_CONSUMER); + async_exch_t *exch = loc_exchange_begin_blocking(); size_t size = loc_count_namespaces_internal(exch); loc_exchange_end(exch); @@ -694,7 +641,7 @@ size_t loc_count_namespaces(void) size_t loc_count_services(service_id_t ns_handle) { - async_exch_t *exch = loc_exchange_begin_blocking(INTERFACE_LOC_CONSUMER); + async_exch_t *exch = loc_exchange_begin_blocking(); size_t size = loc_count_services_internal(exch, ns_handle); loc_exchange_end(exch); @@ -705,7 +652,7 @@ size_t loc_get_namespaces(loc_sdesc_t **data) { /* Loop until read is succesful */ while (true) { - async_exch_t *exch = loc_exchange_begin_blocking(INTERFACE_LOC_CONSUMER); + async_exch_t *exch = loc_exchange_begin_blocking(); size_t count = loc_count_namespaces_internal(exch); loc_exchange_end(exch); @@ -716,7 +663,7 @@ size_t loc_get_namespaces(loc_sdesc_t **data) if (devs == NULL) return 0; - exch = loc_exchange_begin(INTERFACE_LOC_CONSUMER); + exch = loc_exchange_begin(); ipc_call_t answer; aid_t req = async_send_0(exch, LOC_GET_NAMESPACES, &answer); @@ -754,7 +701,7 @@ size_t loc_get_services(service_id_t ns_handle, loc_sdesc_t **data) { /* Loop until read is succesful */ while (true) { - async_exch_t *exch = loc_exchange_begin_blocking(INTERFACE_LOC_CONSUMER); + async_exch_t *exch = loc_exchange_begin_blocking(); size_t count = loc_count_services_internal(exch, ns_handle); loc_exchange_end(exch); @@ -765,7 +712,7 @@ size_t loc_get_services(service_id_t ns_handle, loc_sdesc_t **data) if (devs == NULL) return 0; - exch = loc_exchange_begin(INTERFACE_LOC_CONSUMER); + exch = loc_exchange_begin(); ipc_call_t answer; aid_t req = async_send_1(exch, LOC_GET_SERVICES, ns_handle, &answer); @@ -802,7 +749,7 @@ size_t loc_get_services(service_id_t ns_handle, loc_sdesc_t **data) static errno_t loc_category_get_ids_once(sysarg_t method, sysarg_t arg1, sysarg_t *id_buf, size_t buf_size, size_t *act_size) { - async_exch_t *exch = loc_exchange_begin_blocking(INTERFACE_LOC_CONSUMER); + async_exch_t *exch = loc_exchange_begin_blocking(); ipc_call_t answer; aid_t req = async_send_1(exch, method, arg1, &answer); diff --git a/uspace/lib/c/include/loc.h b/uspace/lib/c/include/loc.h index a3a357500d..1e339ecddc 100644 --- a/uspace/lib/c/include/loc.h +++ b/uspace/lib/c/include/loc.h @@ -42,8 +42,8 @@ typedef void (*loc_cat_change_cb_t)(void *); -extern async_exch_t *loc_exchange_begin_blocking(iface_t); -extern async_exch_t *loc_exchange_begin(iface_t); +extern async_exch_t *loc_exchange_begin_blocking(void); +extern async_exch_t *loc_exchange_begin(void); extern void loc_exchange_end(async_exch_t *); extern errno_t loc_server_register(const char *, loc_srv_t **); diff --git a/uspace/lib/c/include/types/loc.h b/uspace/lib/c/include/types/loc.h index 76c3926224..ab4fa9ce27 100644 --- a/uspace/lib/c/include/types/loc.h +++ b/uspace/lib/c/include/types/loc.h @@ -35,9 +35,11 @@ #ifndef _LIBC_TYPES_LOC_H_ #define _LIBC_TYPES_LOC_H_ +#include + /** Server register with location service */ typedef struct { - int dummy; + async_sess_t *sess; } loc_srv_t; #endif diff --git a/uspace/lib/c/meson.build b/uspace/lib/c/meson.build index 3da99706cd..3ee9a805c6 100644 --- a/uspace/lib/c/meson.build +++ b/uspace/lib/c/meson.build @@ -1,4 +1,5 @@ # +# Copyright (c) 2023 Jiri Svoboda # Copyright (c) 2005 Martin Decky # Copyright (c) 2007 Jakub Jermar # All rights reserved. @@ -185,6 +186,7 @@ test_src = files( 'test/imath.c', 'test/inttypes.c', 'test/io/table.c', + 'test/loc.c', 'test/main.c', 'test/mem.c', 'test/perf.c', diff --git a/uspace/lib/c/test/loc.c b/uspace/lib/c/test/loc.c new file mode 100644 index 0000000000..471a0eeb71 --- /dev/null +++ b/uspace/lib/c/test/loc.c @@ -0,0 +1,85 @@ +/* + * Copyright (c) 2023 Jiri Svoboda + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * - Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * - The name of the author may not be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include +#include +#include + +PCUT_INIT; + +PCUT_TEST_SUITE(loc); + +/** loc_server_register() can be called multiple times */ +PCUT_TEST(server_register) +{ + errno_t rc; + loc_srv_t *sa, *sb; + service_id_t svca, svcb; + char *na, *nb; + char *sna, *snb; + + rc = loc_server_register("test-a", &sa); + PCUT_ASSERT_ERRNO_VAL(EOK, rc); + + rc = loc_server_register("test-b", &sb); + PCUT_ASSERT_ERRNO_VAL(EOK, rc); + + // XXX Without a unique name this is not reentrant + rc = loc_service_register(sa, "test/libc-service-a", &svca); + PCUT_ASSERT_ERRNO_VAL(EOK, rc); + + // XXX Without a unique name this is not reentrant + rc = loc_service_register(sb, "test/libc-service-b", &svcb); + PCUT_ASSERT_ERRNO_VAL(EOK, rc); + + rc = loc_service_get_name(svca, &na); + PCUT_ASSERT_ERRNO_VAL(EOK, rc); + PCUT_ASSERT_STR_EQUALS("test/libc-service-a", na); + + rc = loc_service_get_server_name(svca, &sna); + PCUT_ASSERT_ERRNO_VAL(EOK, rc); + PCUT_ASSERT_STR_EQUALS("test-a", sna); + + rc = loc_service_get_name(svcb, &nb); + PCUT_ASSERT_ERRNO_VAL(EOK, rc); + PCUT_ASSERT_STR_EQUALS("test/libc-service-b", nb); + + rc = loc_service_get_server_name(svcb, &snb); + PCUT_ASSERT_ERRNO_VAL(EOK, rc); + PCUT_ASSERT_STR_EQUALS("test-b", snb); + + rc = loc_service_unregister(sa, svca); + PCUT_ASSERT_ERRNO_VAL(EOK, rc); + rc = loc_service_unregister(sb, svcb); + PCUT_ASSERT_ERRNO_VAL(EOK, rc); + + loc_server_unregister(sa); + loc_server_unregister(sb); +} + +PCUT_EXPORT(loc); diff --git a/uspace/lib/c/test/main.c b/uspace/lib/c/test/main.c index 9f9e27d702..4006a6fe5b 100644 --- a/uspace/lib/c/test/main.c +++ b/uspace/lib/c/test/main.c @@ -1,4 +1,5 @@ /* + * Copyright (c) 2023 Jiri Svoboda * Copyright (c) 2014 Vojtech Horky * All rights reserved. * @@ -41,6 +42,7 @@ PCUT_IMPORT(gsort); PCUT_IMPORT(ieee_double); PCUT_IMPORT(imath); PCUT_IMPORT(inttypes); +PCUT_IMPORT(loc); PCUT_IMPORT(mem); PCUT_IMPORT(odict); PCUT_IMPORT(perf);