From f48bbd3d1c1e2ad82dbf9516d40677ca1dc7b4d5 Mon Sep 17 00:00:00 2001 From: Frans Faase Date: Fri, 13 Sep 2019 11:45:14 +0200 Subject: [PATCH] Initializing domain with configuration as a string For targets that do not support ddsrt_setenv and ddsrt_getenv, an alternative method is needed to supply an application specific configuration. One way to implement this, is to add a function for creating a domain with a string arguments, which needs to be called before any call to dds_create_participant for given domain identifier. The function dds_create_domain has been added, which has as arguments a domain identifier and a configuration string. The string is treated in the same way as the string that is retrieved from the environment variable, in that it may containt a comma separated list of file names and/or XML fragments for the configuration. Two tests have been added. One limits the number of participants to two and verifies that creating a third participant fails. The other tests checks incorrect calls to dds_create_domain. An assert in dds_handle_delete has been weakened. Signed-off-by: Frans Faase --- src/core/ddsc/include/dds/dds.h | 29 ++++- src/core/ddsc/src/dds__domain.h | 2 +- src/core/ddsc/src/dds_domain.c | 39 +++++-- src/core/ddsc/src/dds_handles.c | 2 +- src/core/ddsc/src/dds_participant.c | 7 +- src/core/ddsc/tests/config.c | 43 ++++++++ src/core/ddsi/include/dds/ddsi/q_config.h | 2 +- src/core/ddsi/src/q_config.c | 124 +++++++++++----------- 8 files changed, 173 insertions(+), 75 deletions(-) diff --git a/src/core/ddsc/include/dds/dds.h b/src/core/ddsc/include/dds/dds.h index 884eb3f..6c9ae53 100644 --- a/src/core/ddsc/include/dds/dds.h +++ b/src/core/ddsc/include/dds/dds.h @@ -726,7 +726,7 @@ dds_set_listener(dds_entity_t entity, const dds_listener_t * listener); * If no configuration file exists, the default domain is configured as 0. * * - * @param[in] domain The domain in which to create the participant (can be DDS_DOMAIN_DEFAULT). Valid values for domain id are between 0 and 230. DDS_DOMAIN_DEFAULT is for using the domain in the configuration. + * @param[in] domain The domain in which to create the participant (can be DDS_DOMAIN_DEFAULT). DDS_DOMAIN_DEFAULT is for using the domain in the configuration. * @param[in] qos The QoS to set on the new participant (can be NULL). * @param[in] listener Any listener functions associated with the new participant (can be NULL). @@ -743,6 +743,33 @@ dds_create_participant( const dds_qos_t *qos, const dds_listener_t *listener); +/** + * @brief Creates a domain with a given configuration + * + * To explicitly create a domain based on a configuration passed as a string. + * Normally, the domain is created implicitly on the first call to + * dds_create_particiant based on the configuration specified throught + * the environment. This function allows to by-pass this behaviour. + * + * + * @param[in] domain The domain to be created. DEFAULT_DOMAIN is not allowed. + * @param[in] config A configuration string containing file names and/or XML fragments representing the configuration. + * + * @returns A return code + * + * @retval DDS_RETCODE_OK + * The domain with the domain identifier has been created from + * given configuration string. + * @retval DDS_RETCODE_BAD_PARAMETER + * Illegal value for domain id or the configfile parameter is NULL. + * @retval DDS_PRECONDITION_NOT_MET + * The domain already existed and cannot be created again. + * @retval DDS_RETCODE_ERROR + * An internal error has occurred. + */ +DDS_EXPORT dds_return_t +dds_create_domain(const dds_domainid_t domain, const char *config); + /** * @brief Get entity parent. * diff --git a/src/core/ddsc/src/dds__domain.h b/src/core/ddsc/src/dds__domain.h index 63c2c1b..db5d7bc 100644 --- a/src/core/ddsc/src/dds__domain.h +++ b/src/core/ddsc/src/dds__domain.h @@ -18,7 +18,7 @@ extern "C" { #endif -DDS_EXPORT dds_return_t dds_domain_create (dds_domain **domain_out, dds_domainid_t id); +DDS_EXPORT dds_return_t dds_domain_create_internal (dds_domain **domain_out, dds_domainid_t id, bool use_existing, const char *config) ddsrt_nonnull((1,4)); DDS_EXPORT dds_domain *dds_domain_find_locked (dds_domainid_t id); #if defined (__cplusplus) diff --git a/src/core/ddsc/src/dds_domain.c b/src/core/ddsc/src/dds_domain.c index 601cb96..9bfb67c 100644 --- a/src/core/ddsc/src/dds_domain.c +++ b/src/core/ddsc/src/dds_domain.c @@ -11,7 +11,6 @@ */ #include -#include "dds/ddsrt/environ.h" #include "dds/ddsrt/process.h" #include "dds/ddsrt/heap.h" #include "dds__init.h" @@ -28,7 +27,6 @@ #include "dds/ddsi/q_config.h" #include "dds/ddsi/q_gc.h" #include "dds/ddsi/q_globals.h" -#include "dds/version.h" static dds_return_t dds_domain_free (dds_entity *vdomain); @@ -50,7 +48,7 @@ static int dds_domain_compare (const void *va, const void *vb) static const ddsrt_avl_treedef_t dds_domaintree_def = DDSRT_AVL_TREEDEF_INITIALIZER ( offsetof (dds_domain, m_node), offsetof (dds_domain, m_id), dds_domain_compare, 0); -static dds_return_t dds_domain_init (dds_domain *domain, dds_domainid_t domain_id) +static dds_return_t dds_domain_init (dds_domain *domain, dds_domainid_t domain_id, const char *config) { dds_return_t ret = DDS_RETCODE_OK; char * uri = NULL; @@ -89,8 +87,7 @@ static dds_return_t dds_domain_init (dds_domain *domain, dds_domainid_t domain_i a value (if nothing has been set previously, it a warning is good enough) */ - (void) ddsrt_getenv ("CYCLONEDDS_URI", &uri); - domain->cfgst = config_init (uri, &domain->gv.config, domain_id); + domain->cfgst = config_init (config, &domain->gv.config, domain_id); if (domain->cfgst == NULL) { DDS_ILOG (DDS_LC_CONFIG, domain_id, "Failed to parse configuration XML file %s\n", uri); @@ -197,7 +194,7 @@ dds_domain *dds_domain_find_locked (dds_domainid_t id) return ddsrt_avl_lookup (&dds_domaintree_def, &dds_global.m_domains, &id); } -dds_return_t dds_domain_create (dds_domain **domain_out, dds_domainid_t id) +dds_return_t dds_domain_create_internal (dds_domain **domain_out, dds_domainid_t id, bool use_existing, const char *config) { struct dds_domain *dom; dds_return_t ret; @@ -223,6 +220,11 @@ dds_return_t dds_domain_create (dds_domain **domain_out, dds_domainid_t id) switch (ret) { case DDS_RETCODE_OK: + if (!use_existing) + { + ret = DDS_RETCODE_PRECONDITION_NOT_MET; + break; + } ddsrt_mutex_lock (&dom->m_entity.m_mutex); if (dds_handle_is_closed (&dom->m_entity.m_hdllink)) { @@ -239,7 +241,7 @@ dds_return_t dds_domain_create (dds_domain **domain_out, dds_domainid_t id) break; case DDS_RETCODE_NOT_FOUND: dom = dds_alloc (sizeof (*dom)); - if ((ret = dds_domain_init (dom, id)) < 0) + if ((ret = dds_domain_init (dom, id, config)) < 0) dds_free (dom); else { @@ -256,6 +258,29 @@ dds_return_t dds_domain_create (dds_domain **domain_out, dds_domainid_t id) return ret; } +dds_return_t dds_create_domain(const dds_domainid_t domain, const char *config) +{ + dds_domain *dom; + dds_entity_t ret; + + if (domain == DDS_DOMAIN_DEFAULT || config == NULL) + return DDS_RETCODE_BAD_PARAMETER; + + /* Make sure DDS instance is initialized. */ + if ((ret = dds_init ()) < 0) + goto err_dds_init; + + if ((ret = dds_domain_create_internal (&dom, domain, false, config)) < 0) + goto err_domain_create; + + return DDS_RETCODE_OK; + +err_domain_create: + dds_delete_impl_pinned (&dds_global.m_entity, DIS_EXPLICIT); +err_dds_init: + return ret; +} + static dds_return_t dds_domain_free (dds_entity *vdomain) { struct dds_domain *domain = (struct dds_domain *) vdomain; diff --git a/src/core/ddsc/src/dds_handles.c b/src/core/ddsc/src/dds_handles.c index 17b1f89..c4f1f6c 100644 --- a/src/core/ddsc/src/dds_handles.c +++ b/src/core/ddsc/src/dds_handles.c @@ -172,8 +172,8 @@ int32_t dds_handle_delete (struct dds_handle_link *link) { assert (cf & HDL_FLAG_CLOSING); assert (cf & HDL_FLAG_CLOSED); + assert ((cf & HDL_REFCOUNT_MASK) == 0u); } - assert ((cf & HDL_REFCOUNT_MASK) == 0u); assert ((cf & HDL_PINCOUNT_MASK) == 1u); #endif ddsrt_mutex_lock (&handles.lock); diff --git a/src/core/ddsc/src/dds_participant.c b/src/core/ddsc/src/dds_participant.c index d15f60c..4825f23 100644 --- a/src/core/ddsc/src/dds_participant.c +++ b/src/core/ddsc/src/dds_participant.c @@ -12,11 +12,13 @@ #include #include "dds/ddsrt/cdtors.h" +#include "dds/ddsrt/environ.h" #include "dds/ddsi/q_entity.h" #include "dds/ddsi/q_thread.h" #include "dds/ddsi/q_config.h" #include "dds/ddsi/q_plist.h" #include "dds/ddsi/q_globals.h" +#include "dds/version.h" #include "dds__init.h" #include "dds__domain.h" #include "dds__participant.h" @@ -82,12 +84,15 @@ dds_entity_t dds_create_participant (const dds_domainid_t domain, const dds_qos_ dds_participant * pp; nn_plist_t plist; dds_qos_t *new_qos = NULL; + char *config = ""; /* Make sure DDS instance is initialized. */ if ((ret = dds_init ()) < 0) goto err_dds_init; - if ((ret = dds_domain_create (&dom, domain)) < 0) + (void) ddsrt_getenv (DDS_PROJECT_NAME_NOSPACE_CAPS"_URI", &config); + + if ((ret = dds_domain_create_internal (&dom, domain, true, config)) < 0) goto err_domain_create; new_qos = dds_create_qos (); diff --git a/src/core/ddsc/tests/config.c b/src/core/ddsc/tests/config.c index 76f92b0..3c9acb0 100644 --- a/src/core/ddsc/tests/config.c +++ b/src/core/ddsc/tests/config.c @@ -75,3 +75,46 @@ CU_Test(ddsc_config, simple_udp, .init = ddsrt_init, .fini = ddsrt_fini) { dds_delete(participant); } + +CU_Test(ddsc_config, user_config, .init = ddsrt_init, .fini = ddsrt_fini) { + + CU_ASSERT_FATAL(dds_create_domain(1, + "<"DDS_PROJECT_NAME">any" + "2" + "") == DDS_RETCODE_OK); + + dds_entity_t participant_1; + dds_entity_t participant_2; + dds_entity_t participant_3; + + participant_1 = dds_create_participant(1, NULL, NULL); + + CU_ASSERT_FATAL(participant_1 > 0); + + participant_2 = dds_create_participant(1, NULL, NULL); + + CU_ASSERT_FATAL(participant_2 > 0); + + participant_3 = dds_create_participant(1, NULL, NULL); + + CU_ASSERT(participant_3 <= 0); + + dds_delete(participant_3); + dds_delete(participant_2); + dds_delete(participant_1); +} + +CU_Test(ddsc_config, incorrect_config, .init = ddsrt_init, .fini = ddsrt_fini) { + + CU_ASSERT_FATAL(dds_create_domain(1, NULL) == DDS_RETCODE_BAD_PARAMETER); + CU_ASSERT_FATAL(dds_create_domain(1, "any" + "2" + "") == DDS_RETCODE_BAD_PARAMETER); + CU_ASSERT_FATAL(dds_create_domain(2, + "<"DDS_PROJECT_NAME">any" + "2" + "") == DDS_RETCODE_OK); + CU_ASSERT_FATAL(dds_create_domain(2, "") == DDS_RETCODE_PRECONDITION_NOT_MET); +} diff --git a/src/core/ddsi/include/dds/ddsi/q_config.h b/src/core/ddsi/include/dds/ddsi/q_config.h index ffd1ede..f06a8cd 100644 --- a/src/core/ddsi/include/dds/ddsi/q_config.h +++ b/src/core/ddsi/include/dds/ddsi/q_config.h @@ -393,7 +393,7 @@ struct config struct cfgst; -struct cfgst *config_init (const char *configfile, struct config *cfg, uint32_t domid); +struct cfgst *config_init (const char *config, struct config *cfg, uint32_t domid) ddsrt_nonnull((1,2)); void config_print_cfgst (struct cfgst *cfgst, const struct ddsrt_log_cfg *logcfg); void config_free_source_info (struct cfgst *cfgst); void config_fini (struct cfgst *cfgst); diff --git a/src/core/ddsi/src/q_config.c b/src/core/ddsi/src/q_config.c index e386ea2..9ea3ef0 100644 --- a/src/core/ddsi/src/q_config.c +++ b/src/core/ddsi/src/q_config.c @@ -2696,10 +2696,13 @@ static FILE *config_open_file (char *tok, char **cursor, uint32_t domid) return fp; } -struct cfgst *config_init (const char *configfile, struct config *cfg, uint32_t domid) +struct cfgst *config_init (const char *config, struct config *cfg, uint32_t domid) { int ok = 1; struct cfgst *cfgst; + char env_input[32]; + char *copy, *cursor; + struct ddsrt_xmlp_callbacks cb; memset (cfg, 0, sizeof (*cfg)); @@ -2720,72 +2723,67 @@ struct cfgst *config_init (const char *configfile, struct config *cfg, uint32_t ends up on the right value */ cfgst->cfg->domainId = domid; - /* configfile == NULL will get you the default configuration */ - if (configfile) { - char env_input[32]; - char *copy = ddsrt_strdup (configfile), *cursor = copy; - struct ddsrt_xmlp_callbacks cb; + cb.attr = proc_attr; + cb.elem_close = proc_elem_close; + cb.elem_data = proc_elem_data; + cb.elem_open = proc_elem_open; + cb.error = proc_error; - cb.attr = proc_attr; - cb.elem_close = proc_elem_close; - cb.elem_data = proc_elem_data; - cb.elem_open = proc_elem_open; - cb.error = proc_error; - - while (*cursor && (isspace ((unsigned char) *cursor) || *cursor == ',')) - cursor++; - while (ok && cursor && cursor[0]) + copy = ddsrt_strdup (config); + cursor = copy; + while (*cursor && (isspace ((unsigned char) *cursor) || *cursor == ',')) + cursor++; + while (ok && cursor && cursor[0]) + { + struct ddsrt_xmlp_state *qx; + FILE *fp; + char *tok; + tok = cursor; + if (tok[0] == '<') { - struct ddsrt_xmlp_state *qx; - FILE *fp; - char *tok; - tok = cursor; - if (tok[0] == '<') - { - /* Read XML directly from input string */ - qx = ddsrt_xmlp_new_string (tok, cfgst, &cb); - ddsrt_xmlp_set_options (qx, DDSRT_XMLP_ANONYMOUS_CLOSE_TAG | DDSRT_XMLP_MISSING_CLOSE_AS_EOF); - fp = NULL; - snprintf (env_input, sizeof (env_input), "CYCLONEDDS_URI+%u", (unsigned) (tok - copy)); - cfgst->input = env_input; - cfgst->line = 1; - } - else if ((fp = config_open_file (tok, &cursor, domid)) == NULL) - { - ddsrt_free (copy); - goto error; - } - else - { - qx = ddsrt_xmlp_new_file (fp, cfgst, &cb); - cfgst->input = tok; - cfgst->line = 1; - } - - cfgst->implicit_toplevel = (fp == NULL) ? ITL_ALLOWED : ITL_DISALLOWED; - cfgst->first_data_in_source = true; - cfgst_push (cfgst, 0, &root_cfgelem, cfgst->cfg); - ok = (ddsrt_xmlp_parse (qx) >= 0) && !cfgst->error; - assert (!ok || - (cfgst->path_depth == 1 && cfgst->implicit_toplevel == ITL_DISALLOWED) || - (cfgst->path_depth == 1 + (int) cfgst->implicit_toplevel)); - /* Pop until stack empty: error handling is rather brutal */ - while (cfgst->path_depth > 0) - cfgst_pop (cfgst); - if (fp != NULL) - fclose (fp); - else if (ok) - cursor = tok + ddsrt_xmlp_get_bufpos (qx); - ddsrt_xmlp_free (qx); - assert (fp == NULL || cfgst->implicit_toplevel <= ITL_ALLOWED); - if (cursor) - { - while (*cursor && (isspace ((unsigned char) cursor[0]) || cursor[0] == ',')) - cursor++; - } + /* Read XML directly from input string */ + qx = ddsrt_xmlp_new_string (tok, cfgst, &cb); + ddsrt_xmlp_set_options (qx, DDSRT_XMLP_ANONYMOUS_CLOSE_TAG | DDSRT_XMLP_MISSING_CLOSE_AS_EOF); + fp = NULL; + snprintf (env_input, sizeof (env_input), "CYCLONEDDS_URI+%u", (unsigned) (tok - copy)); + cfgst->input = env_input; + cfgst->line = 1; + } + else if ((fp = config_open_file (tok, &cursor, domid)) == NULL) + { + ddsrt_free (copy); + goto error; + } + else + { + qx = ddsrt_xmlp_new_file (fp, cfgst, &cb); + cfgst->input = tok; + cfgst->line = 1; + } + + cfgst->implicit_toplevel = (fp == NULL) ? ITL_ALLOWED : ITL_DISALLOWED; + cfgst->first_data_in_source = true; + cfgst_push (cfgst, 0, &root_cfgelem, cfgst->cfg); + ok = (ddsrt_xmlp_parse (qx) >= 0) && !cfgst->error; + assert (!ok || + (cfgst->path_depth == 1 && cfgst->implicit_toplevel == ITL_DISALLOWED) || + (cfgst->path_depth == 1 + (int) cfgst->implicit_toplevel)); + /* Pop until stack empty: error handling is rather brutal */ + while (cfgst->path_depth > 0) + cfgst_pop (cfgst); + if (fp != NULL) + fclose (fp); + else if (ok) + cursor = tok + ddsrt_xmlp_get_bufpos (qx); + ddsrt_xmlp_free (qx); + assert (fp == NULL || cfgst->implicit_toplevel <= ITL_ALLOWED); + if (cursor) + { + while (*cursor && (isspace ((unsigned char) cursor[0]) || cursor[0] == ',')) + cursor++; } - ddsrt_free (copy); } + ddsrt_free (copy); /* Set defaults for everything not set that we have a default value for, signal errors for things unset but without a default. */