Define a common section name struct, and use pointers to those structs

Will allow slightly faster comparisons in some cases, and allow common module method names to be defined

Author: arr2036

src/lib/server/module.h

@@ -32,7 +32,7 @@ extern "C" {
 
 typedef struct module_s				module_t;
 typedef struct module_state_func_table_s	module_state_func_table_t;
-typedef struct module_method_name_s		module_method_name_t;
+typedef struct module_method_binding_s		module_method_binding_t;
 typedef struct module_instance_s		module_instance_t;
 typedef struct module_thread_instance_s		module_thread_instance_t;
 typedef struct module_list_type_s		module_list_type_t;
@@ -129,6 +129,7 @@ typedef int (*module_thread_detach_t)(module_thread_inst_ctx_t const *mctx);
 #include <freeradius-devel/server/exfile.h>
 #include <freeradius-devel/server/pool.h>
 #include <freeradius-devel/server/request.h>
+#include <freeradius-devel/server/section.h>
 
 #include <freeradius-devel/unlang/action.h>
 #include <freeradius-devel/unlang/call_env.h>
@@ -144,18 +145,21 @@ extern "C" {
  */
 #define MODULE_INSTANCE_LEN_MAX 256
 
+/** Terminate a module binding list
+ */
+#define MODULE_BINDING_TERMINATOR { .section = NULL }
+
 /** Named methods exported by a module
  *
  */
-struct module_method_name_s {
-	char const			*name1;			//!< i.e. "recv", "send", "process"
-	char const			*name2;			//!< The packet type i.e Access-Request, Access-Reject.
+struct module_method_binding_s {
+	fr_dict_t const				**proto;		//!< Only allow this method to be called in this namespace.
 
-	module_method_t			method;			//!< Module method to call
-	call_env_method_t const	* const method_env;		//!< Call specific conf parsing.
-};
+	section_name_t const			*section;		//!< Identifier for a section.
 
-#define MODULE_NAME_TERMINATOR { NULL }
+	module_method_t				method;			//!< Module method to call
+	call_env_method_t const	* const 	method_env;		//!< Call specific conf parsing.
+};
 
 /** Struct exported by a rlm_* module
  *

src/lib/server/module_method.c

@@ -18,7 +18,7 @@
  * $Id$
  *
  * @file src/lib/server/module_method.c
- * @brief Central module_method_name_t definitions
+ * @brief Central module_method_binding_t definitions
  *
  * This file contains common module_method_t structures which may be
  * referenced within a #virtual_server_compile_t and a #module_t.
@@ -33,33 +33,24 @@
  *
  * @copyright 2022 Arran Cudbard-Bell (a.cudbardb@freeradius.org)
  */
-typedef struct {
-	fr_dict_t const		**proto;	//!< If none-null, restrict matches to this protocol.
-						///< i.e. if both the virtual server module_method_name
-                                                ///< and the module method have non-null proto pointers
-                                                ///< then *proto must be equal for the method name to
-                                                ///< match.
+#include <freeradius-devel/server/module_method.h>
 
-	char const		*name1;		//!< module method name1 which is allowed in this section
-	char const		*name2;		//!< module method name2 which is allowed in this section
-} module_method_name_t;
-
-module_method_name_t module_method_ippool_allocate = {
+section_name_t module_method_ippool_allocate = {
 	.name1 = "ippool",
 	.name2 = "allocate"
 };
 
-module_method_name_t module_method_ippool_extend = {
+section_name_t module_method_ippool_extend = {
 	.name1 = "ippool",
 	.name2 = "extend"
 };
 
-module_method_name_t module_method_ippool_mark = {
+section_name_t module_method_ippool_mark = {
 	.name1 = "ippool",
 	.name2 = "mark"
 };
 
-module_method_name_t module_method_ippool_release = {
+section_name_t module_method_ippool_release = {
 	.name1 = "ippool",
 	.name2 = "release"
 };

src/lib/server/module_method.h

@@ -29,36 +29,15 @@ RCSIDH(module_method_h, "$Id$")
 extern "C" {
 #endif
 
-#include <freeradius-devel/util/dict.h>
+#include <freeradius-devel/server/virtual_servers.h>
 
-/** Specifies a module method identifier
- *
- * These are used in module definitions and by virtual servers to find mutually
- * acceptable module methods to call between a virtual server section and the
- * module that's calling it.
- *
- * For example, a `send Access-Accept` compilation structure may also have a
- * `ippool alloc` method associated with it, to instruct any ippool modules to
- * allocate an IP address.
- */
-typedef struct {
-	fr_dict_t const		**proto;	//!< If none-null, restrict matches to this protocol.
-						///< i.e. if both the virtual server module_method_name
-                                                ///< and the module method have non-null proto pointers
-                                                ///< then *proto must be equal for the method name to
-                                                ///< match.
-
-	char const		*name1;		//!< module method name1 which is allowed in this section
-	char const		*name2;		//!< module method name2 which is allowed in this section
-} module_method_name_t;
-
-extern module_method_name_t module_method_ippool_allocate;
+extern section_name_t module_method_ippool_allocate;
 
-extern module_method_name_t module_method_ippool_extend;
+extern section_name_t module_method_ippool_extend;
 
-extern module_method_name_t module_method_ippool_mark;
+extern section_name_t module_method_ippool_mark;
 
-extern module_method_name_t module_method_ippool_release;
+extern section_name_t module_method_ippool_release;
 
 #ifdef __cplusplus
 }

src/lib/server/module_rlm.c

@@ -448,7 +448,7 @@ module_instance_t *module_rlm_by_name_and_method(module_method_t *method, call_e
 	size_t				len;
 	int				j;
 	module_instance_t		*mi;
-	module_method_name_t const	*methods;
+	module_method_binding_t const	*methods;
 	char const			*method_name1, *method_name2;
 	module_rlm_t const		*mrlm;
 
@@ -470,7 +470,7 @@ module_instance_t *module_rlm_by_name_and_method(module_method_t *method, call_e
 	 */
 	mi = module_rlm_static_by_name(NULL, name);
 	if (mi) {
-		virtual_server_method_t const	*allowed_list;
+		section_name_t const	**allowed_list;
 
 		if (!method) return mi;
 
@@ -481,20 +481,20 @@ module_instance_t *module_rlm_by_name_and_method(module_method_t *method, call_e
 		 *	module has no named methods.  Try to return a
 		 *	method based on the component.
 		 */
-		if (!method_name1 || !mrlm->method_names) goto return_component;
+		if (!method_name1 || !mrlm->bindings) goto return_component;
 
 		/*
 		 *	Walk through the module, finding a matching
 		 *	method.
 		 */
-		for (j = 0; mrlm->method_names[j].name1 != NULL; j++) {
-			methods = &mrlm->method_names[j];
+		for (j = 0; mrlm->bindings[j].section; j++) {
+			methods = &mrlm->bindings[j];
 
 			/*
 			 *	Wildcard match name1, we're
 			 *	done.
 			 */
-			if (methods->name1 == CF_IDENT_ANY) {
+			if (methods->section->name1 == CF_IDENT_ANY) {
 			found:
 				*method = methods->method;
 				if (method_env) *method_env = methods->method_env;
@@ -506,26 +506,26 @@ module_instance_t *module_rlm_by_name_and_method(module_method_t *method, call_e
 			/*
 			 *	If name1 doesn't match, skip it.
 			 */
-			if (strcasecmp(methods->name1, method_name1) != 0) continue;
+			if (strcasecmp(methods->section->name1, method_name1) != 0) continue;
 
 			/*
 			 *	The module can declare a
 			 *	wildcard for name2, in which
 			 *	case it's a match.
 			 */
-			if (methods->name2 == CF_IDENT_ANY) goto found;
+			if (methods->section->name2 == CF_IDENT_ANY) goto found;
 
 			/*
 			 *	No name2 is also a match to no name2.
 			 */
-			if (!methods->name2 && !method_name2) goto found;
+			if (!methods->section->name2 && !method_name2) goto found;
 
 			/*
 			 *	Don't do strcmp on NULLs
 			 */
-			if (!methods->name2 || !method_name2) continue;
+			if (!methods->section->name2 || !method_name2) continue;
 
-			if (strcasecmp(methods->name2, method_name2) == 0) goto found;
+			if (strcasecmp(methods->section->name2, method_name2) == 0) goto found;
 		}
 
 		if (!vs) goto skip_section_method;
@@ -558,22 +558,22 @@ module_instance_t *module_rlm_by_name_and_method(module_method_t *method, call_e
 		 *	then any module method would match, which is
 		 *	bad.
 		 */
-		for (j = 0; allowed_list[j].name1 != NULL; j++) {
+		for (j = 0; allowed_list[j]; j++) {
 			int k;
-			virtual_server_method_t const *allowed = &allowed_list[j];
+			section_name_t const *allowed = allowed_list[j];
 
-			for (k = 0; mrlm->method_names[k].name1 != NULL; k++) {
-				methods = &mrlm->method_names[k];
+			for (k = 0; mrlm->bindings[k].section; k++) {
+				methods = &mrlm->bindings[k];
 
-				fr_assert(methods->name1 != CF_IDENT_ANY); /* should have been caught above */
+				fr_assert(methods->section->name1 != CF_IDENT_ANY); /* should have been caught above */
 
-				if (strcasecmp(methods->name1, allowed->name1) != 0) continue;
+				if (strcasecmp(methods->section->name1, allowed->name1) != 0) continue;
 
 				/*
 				 *	The module matches "recv *",
 				 *	call this method.
 				 */
-				if (methods->name2 == CF_IDENT_ANY) {
+				if (methods->section->name2 == CF_IDENT_ANY) {
 				found_allowed:
 					*method = methods->method;
 					return mi;
@@ -582,14 +582,14 @@ module_instance_t *module_rlm_by_name_and_method(module_method_t *method, call_e
 				/*
 				 *	No name2 is also a match to no name2.
 				 */
-				if (!methods->name2 && !allowed->name2) goto found_allowed;
+				if (!methods->section->name2 && !allowed->name2) goto found_allowed;
 
 				/*
 				 *	Don't do strcasecmp on NULLs
 				 */
-				if (!methods->name2 || !allowed->name2) continue;
+				if (!methods->section->name2 || !allowed->name2) continue;
 
-				if (strcasecmp(methods->name2, allowed->name2) == 0) goto found_allowed;
+				if (strcasecmp(methods->section->name2, allowed->name2) == 0) goto found_allowed;
 			}
 		}
 
@@ -663,7 +663,7 @@ module_instance_t *module_rlm_by_name_and_method(module_method_t *method, call_e
 	/*
 	 *	We've found the module, but it has no named methods.
 	 */
-	if (!mrlm->method_names) {
+	if (!mrlm->bindings) {
 		*name1 = name + (p - inst_name);
 		*name2 = NULL;
 		goto finish;
@@ -675,24 +675,24 @@ module_instance_t *module_rlm_by_name_and_method(module_method_t *method, call_e
 	 *	matches anything else.
 	 */
 	if (!q) {
-		for (j = 0; mrlm->method_names[j].name1 != NULL; j++) {
-			methods = &mrlm->method_names[j];
+		for (j = 0; mrlm->bindings[j].section; j++) {
+			methods = &mrlm->bindings[j];
 
 			/*
 			 *	If we do not have the second $method, then ignore it!
 			 */
-			if (methods->name2 && (methods->name2 != CF_IDENT_ANY)) continue;
+			if (methods->section->name2 && (methods->section->name2 != CF_IDENT_ANY)) continue;
 
 			/*
 			 *	Wildcard match name1, we're
 			 *	done.
 			 */
-			if (!methods->name1 || (methods->name1 == CF_IDENT_ANY)) goto found_name1;
+			if (!methods->section->name1 || (methods->section->name1 == CF_IDENT_ANY)) goto found_name1;
 
 			/*
 			 *	If name1 doesn't match, skip it.
 			 */
-			if (strcasecmp(methods->name1, p) != 0) continue;
+			if (strcasecmp(methods->section->name1, p) != 0) continue;
 
 		found_name1:
 			/*
@@ -729,41 +729,41 @@ module_instance_t *module_rlm_by_name_and_method(module_method_t *method, call_e
 	 *
 	 *	Loop over the method names, seeing if we have a match.
 	 */
-	for (j = 0; mrlm->method_names[j].name1 != NULL; j++) {
-		methods = &mrlm->method_names[j];
+	for (j = 0; mrlm->bindings[j].section; j++) {
+		methods = &mrlm->bindings[j];
 
 		/*
 		 *	If name1 doesn't match, skip it.
 		 */
-		if (strncasecmp(methods->name1, p, len) != 0) continue;
+		if (strncasecmp(methods->section->name1, p, len) != 0) continue;
 
 		/*
 		 *	It may have been a partial match, like "rec",
 		 *	instead of "recv".  In which case check if it
 		 *	was a FULL match.
 		 */
-		if (strlen(methods->name1) != len) continue;
+		if (strlen(methods->section->name1) != len) continue;
 
 		/*
 		 *	The module can declare a
 		 *	wildcard for name2, in which
 		 *	case it's a match.
 		 */
-		if (!methods->name2 || (methods->name2 == CF_IDENT_ANY)) goto found_name2;
+		if (!methods->section->name2 || (methods->section->name2 == CF_IDENT_ANY)) goto found_name2;
 
 		/*
 		 *	Don't do strcmp on NULLs
 		 */
-		if (!methods->name2) continue;
+		if (!methods->section->name2) continue;
 
-		if (strcasecmp(methods->name2, q) != 0) continue;
+		if (strcasecmp(methods->section->name2, q) != 0) continue;
 
 	found_name2:
 		/*
 		 *	Update name1/name2 with the methods
 		 *	that were found.
 		 */
-		*name1 = methods->name1;
+		*name1 = methods->section->name1;
 		*name2 = name + (q - inst_name);
 		*method = methods->method;
 		if (method_env) *method_env = methods->method_env;

src/lib/server/module_rlm.h

@@ -34,8 +34,7 @@ extern "C" {
 
 typedef struct {
 	module_t			common;			//!< Common fields presented by all modules.
-	module_method_name_t const	*method_names;		//!< named methods
-	fr_dict_t const			**dict;			//!< pointer to local fr_dict_t*
+	module_method_binding_t const	*bindings;		//!< named methods
 } module_rlm_t;
 
 /** Cast a module_t to a module_rlm_t

src/lib/server/process.h

@@ -467,20 +467,17 @@ static inline unlang_action_t new_client(rlm_rcode_t *p_result, module_ctx_t con
 
 #define DYNAMIC_CLIENT_SECTIONS \
 	{ \
-		.name1 = "new", \
-		.name2 = "client", \
+		.section = SECTION_NAME("new", "client"), \
 		.actions = &mod_actions_authorize, \
 		.offset = PROCESS_CONF_OFFSET(new_client), \
 	}, \
 	{ \
-		.name1 = "add", \
-		.name2 = "client", \
+		.section = SECTION_NAME("add", "client"), \
 		.actions = &mod_actions_authorize, \
 		.offset = PROCESS_CONF_OFFSET(add_client), \
 	}, \
 	{ \
-		.name1 = "deny", \
-		.name2 = "client", \
+		.section = SECTION_NAME("deny", "client"), \
 		.actions = &mod_actions_authorize, \
 		.offset = PROCESS_CONF_OFFSET(deny_client), \
 	}