Tidy up module loading code amdj/modules

The existing code had several defects:

- Core module file names were hardcoded, while autoloaded non-
  core module filenames were not.  Code that loaded core modules
  had a different but in some places duplicated structure to the
  code that loaded autoloaded modules.

- Module loading for autoloaded modules happened in dentry
  order, making it unpredictable from one server to the next, or
  even across instances of `make install` on the same server.

- Module loading for autoloaded modules did not verify that the
  dentry was a file before trying to load it.

- An IRCd start (or a conftest) only printed lines about loading
  modules mentioned in the configuration file.

- The configuration file was parsed before loading core and
  autoloaded modules, meaning any modules specified in the
  configuration file would be loaded first.

- Once a module was loaded, it was added to the head of the
  module list, making MODLIST show them in reverse loaded order
  (newest (re)loaded modules at the top of the list).

These are addressed as follows:

- We now scan the core and autoload module directories and use
  this to determine which modules to load.

- We now sort the dentries according to their filename before
  iterating them to load them, making module load order finally
  deterministic.

- We now test if a dentry is actually a file before trying to
  load it.

- We now print a message to the console (if running in
  foreground or conftest mode) when loading any module,
  including core and autoloaded modules.

- The message printed to the console now has the IRCd module
  directory prefix stripped from it if it matches.

- The configuration file is now parsed after loading core and
  autoload modules.

- The modules are added to the module list in the order that
  they are loaded. This now makes MODLIST show the order that
  modules were (re)loaded in.

Modifications to two modules are part of this effort:

- The autoload module `m_alias` relied upon the alias dict not
  being NULL when loaded, but this is only the case after the
  configuration file has been parsed.

  The module already had a "rehash" hook to destroy all existing
  aliases and create new ones, so we can just make the module do
  nothing on load (remove its modinit function) and change it to
  use the "conf_read_end" hook instead of the "rehash" hook, and
  it will still create aliases after the configuration file is
  subsequently parsed.

  With this modification, there are now no in-tree consumers of
  the "rehash" hook.

- The autoload module `m_services` iterated `service_list` upon
  loading it, which will now be empty (a no-op) since the module
  is loaded before configuration file is parsed.

  This module too handles the configuration file being parsed
  with a "conf_read_end" hook and takes the appropriate action,
  so we can just remove this code from its modinit function.

All other modules' modinit functions have been audited for
behaviour like this; there was nothing of note.

6 files changed, 138 insertions, 83 deletions

diff --git a/include/defaults.h b/include/defaults.h
index 923af201..4aedf27a 100644
--- a/include/defaults.h
+++ b/include/defaults.h
@@ -104,7 +104,7 @@ extern const char *ircd_paths[IRCD_PATH_COUNT];
 #define DPATH		IRCD_PREFIX
 #define BINPATH		IRCD_PREFIX "/bin/"
 #define MODPATH		MODULE_DIR
-#define AUTOMODPATH	MODULE_DIR "/autoload/"
+#define AUTOMODPATH	MODULE_DIR "/autoload"
 #define ETCPATH		ETC_DIR
 #define LOGPATH		LOG_DIR
 #define UHPATH		HELP_DIR "/users"
diff --git a/include/modules.h b/include/modules.h
index 41d0f658..cb239ee2 100644
--- a/include/modules.h
+++ b/include/modules.h
@@ -140,9 +140,6 @@ extern void load_module(char *path);
 /* load all modules */
 extern void load_all_modules(bool warn);
 
-/* load core modules */
-extern void load_core_modules(bool);
-
 extern bool unload_one_module(const char *, bool);
 extern bool load_one_module(const char *, int, bool);
 extern bool load_a_module(const char *, bool, int, bool);
diff --git a/ircd/ircd.c b/ircd/ircd.c
index 8b2aeee2..ec233d95 100644
--- a/ircd/ircd.c
+++ b/ircd/ircd.c
@@ -137,7 +137,7 @@ const char *ircd_paths[IRCD_PATH_COUNT] = {
 
 const char *ircd_pathnames[IRCD_PATH_COUNT] = {
 	[IRCD_PATH_PREFIX] = "prefix",
-	[IRCD_PATH_MODULES] = "modules",
+	[IRCD_PATH_MODULES] = "core modules",
 	[IRCD_PATH_AUTOLOAD_MODULES] = "autoload modules",
 	[IRCD_PATH_ETC] = "config",
 	[IRCD_PATH_LOG] = "log",
@@ -658,10 +658,9 @@ solanum_main(int argc, char * const argv[])
 
 	if (testing_conf)
 		fprintf(stderr, "\nBeginning config test\n");
-	read_conf_files(true);	/* cold start init conf files */
 
 	load_all_modules(1);
-	load_core_modules(1);
+	read_conf_files(true);	/* cold start init conf files */
 
 	init_isupport();
 
diff --git a/ircd/modules.c b/ircd/modules.c
index 93e43acc..e6bf7693 100644
--- a/ircd/modules.c
+++ b/ircd/modules.c
@@ -44,28 +44,14 @@
 #	error "Solanum requires loadable module support."
 #endif
 
+struct module_dentry
+{
+	char name[PATH_MAX];
+};
+
 rb_dlink_list module_list;
 rb_dlink_list mod_paths;
 
-static const char *core_module_table[] = {
-	"m_ban",
-	"m_die",
-	"m_error",
-	"m_identified",
-	"m_join",
-	"m_kick",
-	"m_kill",
-	"m_message",
-	"m_mode",
-	"m_modules",
-	"m_nick",
-	"m_part",
-	"m_quit",
-	"m_server",
-	"m_squit",
-	NULL
-};
-
 #define MOD_WARN_DELTA (90 * 86400)	/* time in seconds, 86400 seconds in a day */
 
 void
@@ -199,69 +185,138 @@ findmodule_byname(const char *name)
 	return NULL;
 }
 
-/* load_all_modules()
+/* module_dentry_sort_cb()
  *
- * input        -
- * output       -
- * side effects -
+ * Helper function for load_all_modules_dir()
+ *
+ * input        - 2 module file names
+ * output       - file name sorting order
+ * side effects - the array of modules is sorted according to the return value
  */
-void
-load_all_modules(bool warn)
+static int
+module_dentry_sort_cb(const void *ptr1, const void *ptr2)
 {
-	DIR *system_module_dir = NULL;
-	struct dirent *ldirent = NULL;
-	char module_fq_name[PATH_MAX + 1];
-	size_t module_ext_len = strlen(LT_MODULE_EXT);
+	const struct module_dentry *mod1 = ptr1;
+	const struct module_dentry *mod2 = ptr2;
+
+	return strcmp(mod1->name, mod2->name);
+}
 
-	system_module_dir = opendir(ircd_paths[IRCD_PATH_AUTOLOAD_MODULES]);
+/* load_all_modules_dir()
+ *
+ * Helper function for load_all_modules()
+ *
+ * input        - Module directory, whether this directory is for core modules or not,
+ *                whether to warn about module loading
+ * output       - None
+ * side effects - Loads every module located in the directory. If it is a core module
+ *                and it fails to load, the process exits in failure.
+ */
+static void
+load_all_modules_dir(const char *path, bool core, bool warn)
+{
+	const size_t module_ext_len = strlen(LT_MODULE_EXT);
+	struct module_dentry *dentries = NULL;
+	size_t dentries_length = 0;
+	size_t dentries_used = 0;
+	char modpath[PATH_MAX + 1];
+	struct dirent *dirent;
+	struct stat statbuf;
+	DIR *moddir;
+	size_t len;
 
-	if(system_module_dir == NULL)
+	if ((moddir = opendir(path)) == NULL)
 	{
-		ilog(L_MAIN, "Could not load modules from %s: %s", ircd_paths[IRCD_PATH_AUTOLOAD_MODULES], strerror(errno));
-		return;
+		if (! core)
+		{
+			ilog(L_MAIN, "Could not load modules from %s (%s)", path, strerror(errno));
+			return;
+		}
+
+		ilog(L_MAIN, "Could not load core modules from %s (%s); terminating ircd", path, strerror(errno));
+		exit(EXIT_FAILURE);
 	}
 
-	while ((ldirent = readdir(system_module_dir)) != NULL)
+	while ((dirent = readdir(moddir)) != NULL)
 	{
-		size_t len = strlen(ldirent->d_name);
+		// File name length is not greater than the size of required extension (e.g. ".so")
+		if ((len = strlen(dirent->d_name)) <= module_ext_len)
+			continue;
+
+		// File name does not end with required extension
+		if (strncasecmp(dirent->d_name + (len - module_ext_len), LT_MODULE_EXT, module_ext_len) != 0)
+			continue;
 
-		if(len > module_ext_len &&
-			rb_strncasecmp(ldirent->d_name + (len - module_ext_len), LT_MODULE_EXT, module_ext_len) == 0)
+		// Unable to obtain dentry info or dentry is not actually a file?
+		snprintf(modpath, sizeof modpath, "%s/%s", path, dirent->d_name);
+		if (! (stat(modpath, &statbuf) == 0 && S_ISREG(statbuf.st_mode)))
+			continue;
+
+		// Extend the modules array if it is not long enough
+		if (dentries_used == dentries_length)
 		{
-			(void) snprintf(module_fq_name, sizeof(module_fq_name), "%s/%s",
-					ircd_paths[IRCD_PATH_AUTOLOAD_MODULES], ldirent->d_name);
-			(void) load_a_module(module_fq_name, warn, MAPI_ORIGIN_CORE, false);
+			dentries_length += 32;
+			dentries = rb_realloc(dentries, dentries_length * sizeof *dentries);
 		}
 
+		// Add this module to the array of modules to load
+		rb_strlcpy(dentries[dentries_used].name, dirent->d_name, sizeof dentries[dentries_used].name);
+
+		// Advance
+		dentries_used++;
 	}
-	(void) closedir(system_module_dir);
-}
 
-/* load_core_modules()
- *
- * input        -
- * output       -
- * side effects - core modules are loaded, if any fail, kill ircd
- */
-void
-load_core_modules(bool warn)
-{
-	char module_name[PATH_MAX];
-	int i;
+	closedir(moddir);
+
+	if (! dentries_used)
+	{
+		if (core)
+		{
+			ilog(L_MAIN, "Could not load core modules from %s (empty); terminating ircd", path);
+			exit(EXIT_FAILURE);
+		}
+
+		return;
+	}
 
+	// Sort the array of modules to load based on their file name
+	qsort(dentries, dentries_used, sizeof(struct module_dentry), &module_dentry_sort_cb);
 
-	for (i = 0; core_module_table[i]; i++)
+	// Attempt to load each module
+	for (size_t i = 0; i < dentries_used; i++)
 	{
-		snprintf(module_name, sizeof(module_name), "%s/%s", ircd_paths[IRCD_PATH_MODULES], core_module_table[i]);
+		char modpath[PATH_MAX + 1];
 
-		if(load_a_module(module_name, warn, MAPI_ORIGIN_CORE, true) == false)
+		snprintf(modpath, sizeof modpath, "%s/%s", path, dentries[i].name);
+
+		if (load_a_module(modpath, warn, MAPI_ORIGIN_CORE, core))
+			continue;
+
+		if (core)
 		{
-			ilog(L_MAIN,
-			     "Error loading core module %s: terminating ircd",
-			     core_module_table[i]);
+			ilog(L_MAIN, "Error while loading core module %s; terminating ircd", dentries[i].name);
 			exit(EXIT_FAILURE);
 		}
+
+		ilog(L_MAIN, "Error while loading module %s", dentries[i].name);
 	}
+
+	// Clean up
+	rb_free(dentries);
+}
+
+/* load_all_modules()
+ *
+ * input        -
+ * output       -
+ * side effects - Loads every core module, then every module in the autoload directory.
+ *                If a core module fails to load, the process exits in failure.
+ */
+void
+load_all_modules(bool warn)
+{
+	load_all_modules_dir(ircd_paths[IRCD_PATH_MODULES], true, warn);
+	load_all_modules_dir(ircd_paths[IRCD_PATH_AUTOLOAD_MODULES], false, warn);
 }
 
 /* load_one_module()
@@ -276,10 +331,7 @@ load_one_module(const char *path, int origin, bool coremodule)
 	char modpath[PATH_MAX];
 	rb_dlink_node *pathst;
 
-	if (server_state_foreground)
-		inotice("loading module %s ...", path);
-
-	if(coremodule)
+	if (coremodule)
 		origin = MAPI_ORIGIN_CORE;
 
 	RB_DLINK_FOREACH(pathst, mod_paths.head)
@@ -452,6 +504,7 @@ unload_one_module(const char *name, bool warn)
 bool
 load_a_module(const char *path, bool warn, int origin, bool core)
 {
+	const char *modpath = path;
 	struct module *mod;
 	lt_dlhandle tmpptr;
 	char *mod_displayname, *c;
@@ -459,6 +512,22 @@ load_a_module(const char *path, bool warn, int origin, bool core)
 
 	int *mapi_version;
 
+	if (server_state_foreground)
+	{
+		// Trim off the module installation prefix if the path matches it
+		if (strncmp(path, ircd_paths[IRCD_PATH_MODULES], strlen(ircd_paths[IRCD_PATH_MODULES])) == 0)
+		{
+			modpath += strlen(ircd_paths[IRCD_PATH_MODULES]);
+
+			if (*modpath == '/')
+				modpath++;
+			else
+				modpath = path;
+		}
+
+		inotice("loading module %s ...", modpath);
+	}
+
 	mod_displayname = rb_basename(path);
 
 	/* Trim off the ending for the display name if we have to */
@@ -690,7 +759,7 @@ load_a_module(const char *path, bool warn, int origin, bool core)
 	mod->mapi_version = MAPI_VERSION(*mapi_version);
 	mod->origin = origin;
 	mod->path = rb_strdup(path);
-	rb_dlinkAdd(mod, &mod->node, &module_list);
+	rb_dlinkAddTail(mod, &mod->node, &module_list);
 
 	if(warn)
 	{
@@ -797,7 +866,6 @@ modules_do_restart(void *unused)
 	}
 
 	load_all_modules(false);
-	load_core_modules(false);
 	rehash(false);
 
 	mod_notify_clicaps();
diff --git a/modules/m_alias.c b/modules/m_alias.c
index 2645f121..614d422d 100644
--- a/modules/m_alias.c
+++ b/modules/m_alias.c
@@ -35,17 +35,16 @@
 
 static const char alias_desc[] = "Provides the system for services aliases";
 
-static int _modinit(void);
 static void _moddeinit(void);
 static void reload_aliases(void *);
 static void m_alias(struct MsgBuf *, struct Client *, struct Client *, int, const char **);
 
 mapi_hfn_list_av1 alias_hfnlist[] = {
-	{ "rehash", reload_aliases },
+	{ "conf_read_end", reload_aliases },
 	{ NULL, NULL },
 };
 
-DECLARE_MODULE_AV2(alias, _modinit, _moddeinit, NULL, NULL, alias_hfnlist, NULL, NULL, alias_desc);
+DECLARE_MODULE_AV2(alias, NULL, _moddeinit, NULL, NULL, alias_hfnlist, NULL, NULL, alias_desc);
 
 static rb_dlink_list alias_messages;
 static const struct MessageEntry alias_msgtab[] =
@@ -87,13 +86,6 @@ destroy_aliases(void)
 	}
 }
 
-static int
-_modinit(void)
-{
-	create_aliases();
-	return 0;
-}
-
 static void
 _moddeinit(void)
 {
diff --git a/modules/m_services.c b/modules/m_services.c
index bf378600..ab845f1e 100644
--- a/modules/m_services.c
+++ b/modules/m_services.c
@@ -100,7 +100,6 @@ DECLARE_MODULE_AV2(services, _modinit, _moddeinit, services_clist, NULL, service
 static int
 _modinit(void)
 {
-	mark_services();
 	add_isupport("FNC", isupport_string, "");
 	return 0;
 }