From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Sun, 2 Apr 2023 19:10:23 +0000 (-0600)
Subject: modules: Bubble up errors so the server will refuse to start if there's a module... 
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9e269ff4b8393c8422f06688f6242b2fe88d3831;p=thirdparty%2Ffreeradius-server.git

modules: Bubble up errors so the server will refuse to start if there's a module error

Decorate all the functions with "warn_unused_result" so this doesn't happen again.
---

diff --git a/src/lib/server/module.h b/src/lib/server/module.h
index 29d09060e4..f29b6018c8 100644
--- a/src/lib/server/module.h
+++ b/src/lib/server/module.h
@@ -401,25 +401,25 @@ typedef struct {
  * @{
  */
 int		module_submodule_parse(UNUSED TALLOC_CTX *ctx, void *out, void *parent,
-				       CONF_ITEM *ci, UNUSED CONF_PARSER const *rule);
+				       CONF_ITEM *ci, UNUSED CONF_PARSER const *rule) CC_HINT(warn_unused_result);
 /** @} */
 
 /** @name Module and module thread lookup
  *
  * @{
  */
-module_instance_t	*module_parent(module_instance_t const *child);
+module_instance_t	*module_parent(module_instance_t const *child) CC_HINT(warn_unused_result);
 
-module_instance_t	*module_root(module_instance_t const *child);
+module_instance_t	*module_root(module_instance_t const *child); CC_HINT(warn_unused_result)
 
 module_instance_t	*module_by_name(module_list_t const *ml, module_instance_t const *parent, char const *asked_name)
-			CC_HINT(nonnull(1,3));
+			CC_HINT(nonnull(1,3)) CC_HINT(warn_unused_result);
 
-module_instance_t	*module_by_data(module_list_t const *ml, void const *data);
+module_instance_t	*module_by_data(module_list_t const *ml, void const *data) CC_HINT(warn_unused_result);
 
-module_thread_instance_t *module_thread(module_instance_t *mi);
+module_thread_instance_t *module_thread(module_instance_t *mi) CC_HINT(warn_unused_result);
 
-module_thread_instance_t *module_thread_by_data(module_list_t const *ml, void const *data);
+module_thread_instance_t *module_thread_by_data(module_list_t const *ml, void const *data) CC_HINT(warn_unused_result);
 /** @} */
 
 /** @name Module and module thread initialisation and instantiation
@@ -430,24 +430,24 @@ void		module_free(module_instance_t *mi);
 
 void		modules_thread_detach(module_list_t const *ml);
 
-int		modules_thread_instantiate(TALLOC_CTX *ctx, module_list_t const *ml, fr_event_list_t *el) CC_HINT(nonnull);
+int		modules_thread_instantiate(TALLOC_CTX *ctx, module_list_t const *ml, fr_event_list_t *el) CC_HINT(nonnull) CC_HINT(warn_unused_result);
 
-int		module_instantiate(module_instance_t *mi) CC_HINT(nonnull);
+int		module_instantiate(module_instance_t *mi) CC_HINT(nonnull) CC_HINT(warn_unused_result);
 
-int		modules_instantiate(module_list_t const *ml) CC_HINT(nonnull);
+int		modules_instantiate(module_list_t const *ml) CC_HINT(nonnull) CC_HINT(warn_unused_result);
 
-int		module_bootstrap(module_instance_t *mi) CC_HINT(nonnull);
+int		module_bootstrap(module_instance_t *mi) CC_HINT(nonnull) CC_HINT(warn_unused_result);
 
-int		modules_bootstrap(module_list_t const *ml) CC_HINT(nonnull);
+int		modules_bootstrap(module_list_t const *ml) CC_HINT(nonnull) CC_HINT(warn_unused_result);
 
-int		module_conf_parse(module_instance_t *mi, CONF_SECTION *mod_cs) CC_HINT(nonnull);
+int		module_conf_parse(module_instance_t *mi, CONF_SECTION *mod_cs) CC_HINT(nonnull) CC_HINT(warn_unused_result);
 
 module_instance_t *module_alloc(module_list_t *ml,
 			        module_instance_t const *parent,
 			        dl_module_type_t type, char const *mod_name, char const *inst_name)
-			        CC_HINT(nonnull(1));
+			        CC_HINT(nonnull(1)) CC_HINT(warn_unused_result);
 
-module_list_t	*module_list_alloc(TALLOC_CTX *ctx, char const *name);
+module_list_t	*module_list_alloc(TALLOC_CTX *ctx, char const *name) CC_HINT(warn_unused_result);
 
 void		modules_init(char const *lib_dir);
 /** @} */
diff --git a/src/lib/server/module_rlm.c b/src/lib/server/module_rlm.c
index 3b8807fb6b..7fd80c073a 100644
--- a/src/lib/server/module_rlm.c
+++ b/src/lib/server/module_rlm.c
@@ -206,7 +206,7 @@ int module_rlm_sibling_section_find(CONF_SECTION **out, CONF_SECTION *module, ch
 			parent = tmp;
 		} while (true);
 
-		module_instantiate(module_by_name(rlm_modules, NULL, inst_name));
+		if (unlikely(module_instantiate(module_by_name(rlm_modules, NULL, inst_name)) < 0)) return -1;
 	}
 
 	/*
@@ -1007,7 +1007,7 @@ int modules_rlm_bootstrap(CONF_SECTION *root)
 	     ci = cf_item_next(modules, ci)) {
 		char const *name;
 		CONF_SECTION *subcs;
-		module_instance_t *mi;
+		module_instance_t *mi = NULL;
 
 		/*
 		 *	@todo - maybe this should be a warning?
@@ -1066,10 +1066,7 @@ int modules_rlm_bootstrap(CONF_SECTION *root)
 		 *	Compile the default "actions" subsection, which includes retries.
 		 */
 		actions = cf_section_find(subcs, "actions", NULL);
-		if (actions && unlang_compile_actions(&mi->actions, actions, (mi->module->type & MODULE_TYPE_RETRY) != 0)) {
-			talloc_free(mi);
-			goto error;
-		}
+		if (actions && unlang_compile_actions(&mi->actions, actions, (mi->module->type & MODULE_TYPE_RETRY) != 0)) goto error;
 	}
 
 	/*
@@ -1077,9 +1074,13 @@ int modules_rlm_bootstrap(CONF_SECTION *root)
 	 *	This needs to be after parsing so that submodules can access
 	 *	their parent's fully parsed data.
 	 */
-	modules_bootstrap(rlm_modules);
+	{
+		int ret = modules_bootstrap(rlm_modules);
 
-	cf_log_debug(modules, " } # modules");
+		cf_log_debug(modules, " } # modules");
+
+		if (unlikely(ret < 0)) return -1;
+	}
 
 	if (fr_command_register_hook(NULL, NULL, modules, module_cmd_list_table) < 0) {
 		PERROR("Failed registering radmin commands for modules");