lib/module.c and callers of frrmod_load(): fix error messages

    frrmod_load() attempts to dlopen() several possible paths
    (constructed from its basename argument) until one succeeds.

    Each dlopen() attempt may fail for a different reason, and
    the important one might not be the last one. Example:

	dlopen(a/foo): file not found
	dlopen(b/foo): symbol "bar" missing
	dlopen(c/foo): file not found

    Previous code reported only the most recent error. Now frrmod_load()
    describes each dlopen() failure.

Signed-off-by: G. Paul Ziemba <paulz@labn.net>

3 files changed, 88 insertions, 22 deletions

diff --git a/lib/libfrr.c b/lib/libfrr.c
index d03437328b..9b05bb4fbf 100644
--- a/lib/libfrr.c
+++ b/lib/libfrr.c
@@ -674,13 +674,19 @@ static void frr_mkdir(const char *path, bool strip)
 			 strerror(errno));
 }
 
+static void _err_print(const void *cookie, const char *errstr)
+{
+	const char *prefix = (const char *)cookie;
+
+	fprintf(stderr, "%s: %s\n", prefix, errstr);
+}
+
 static struct thread_master *master;
 struct thread_master *frr_init(void)
 {
 	struct option_chain *oc;
 	struct frrmod_runtime *module;
 	struct zprivs_ids_t ids;
-	char moderr[256];
 	char p_instance[16] = "", p_pathspace[256] = "";
 	const char *dir;
 	dir = di->module_path ? di->module_path : frr_moduledir;
@@ -734,11 +740,9 @@ struct thread_master *frr_init(void)
 	frrmod_init(di->module);
 	while (modules) {
 		modules = (oc = modules)->next;
-		module = frrmod_load(oc->arg, dir, moderr, sizeof(moderr));
-		if (!module) {
-			fprintf(stderr, "%s\n", moderr);
+		module = frrmod_load(oc->arg, dir, _err_print, __func__);
+		if (!module)
 			exit(1);
-		}
 		XFREE(MTYPE_TMP, oc);
 	}
 
diff --git a/lib/module.c b/lib/module.c
index 1d51a6396d..4037bfbeb0 100644
--- a/lib/module.c
+++ b/lib/module.c
@@ -26,9 +26,11 @@
 #include "module.h"
 #include "memory.h"
 #include "lib/version.h"
+#include "printfrr.h"
 
 DEFINE_MTYPE_STATIC(LIB, MODULE_LOADNAME, "Module loading name");
 DEFINE_MTYPE_STATIC(LIB, MODULE_LOADARGS, "Module loading arguments");
+DEFINE_MTYPE_STATIC(LIB, MODULE_LOAD_ERR, "Module loading error");
 
 static struct frrmod_info frrmod_default_info = {
 	.name = "libfrr",
@@ -67,14 +69,64 @@ void frrmod_init(struct frrmod_runtime *modinfo)
 	execname = modinfo->info->name;
 }
 
-struct frrmod_runtime *frrmod_load(const char *spec, const char *dir, char *err,
-				   size_t err_len)
+/*
+ * If caller wants error strings, it should define non-NULL pFerrlog
+ * which will be called with 0-terminated error messages. These
+ * messages will NOT contain newlines, and the (*pFerrlog)() function
+ * could be called multiple times for a single call to frrmod_load().
+ *
+ * The (*pFerrlog)() function may copy these strings if needed, but
+ * should expect them to be freed by frrmod_load() before frrmod_load()
+ * returns.
+ *
+ * frrmod_load() is coded such that (*pFerrlog)() will be called only
+ * in the case where frrmod_load() returns an error.
+ */
+struct frrmod_runtime *frrmod_load(const char *spec, const char *dir,
+				   void (*pFerrlog)(const void *, const char *),
+				   const void *pErrlogCookie)
 {
 	void *handle = NULL;
 	char name[PATH_MAX], fullpath[PATH_MAX * 2], *args;
 	struct frrmod_runtime *rtinfo, **rtinfop;
 	const struct frrmod_info *info;
 
+#define FRRMOD_LOAD_N_ERRSTR 10
+	char *aErr[FRRMOD_LOAD_N_ERRSTR];
+	unsigned int iErr = 0;
+
+	memset(aErr, 0, sizeof(aErr));
+
+#define ERR_RECORD(...)                                                        \
+	do {                                                                   \
+		if (pFerrlog && (iErr < FRRMOD_LOAD_N_ERRSTR)) {               \
+			aErr[iErr++] = asprintfrr(MTYPE_MODULE_LOAD_ERR,       \
+						  __VA_ARGS__);                \
+		}                                                              \
+	} while (0)
+
+#define ERR_REPORT                                                             \
+	do {                                                                   \
+		if (pFerrlog) {                                                \
+			unsigned int i;                                        \
+                                                                               \
+			for (i = 0; i < iErr; ++i) {                           \
+				(*pFerrlog)(pErrlogCookie, aErr[i]);           \
+			}                                                      \
+		}                                                              \
+	} while (0)
+
+#define ERR_FREE                                                               \
+	do {                                                                   \
+		unsigned int i;                                                \
+                                                                               \
+		for (i = 0; i < iErr; ++i) {                                   \
+			XFREE(MTYPE_MODULE_LOAD_ERR, aErr[i]);                 \
+			aErr[i] = 0;                                           \
+		}                                                              \
+		iErr = 0;                                                      \
+	} while (0)
+
 	snprintf(name, sizeof(name), "%s", spec);
 	args = strchr(name, ':');
 	if (args)
@@ -85,32 +137,41 @@ struct frrmod_runtime *frrmod_load(const char *spec, const char *dir, char *err,
 			snprintf(fullpath, sizeof(fullpath), "%s/%s_%s.so", dir,
 				 execname, name);
 			handle = dlopen(fullpath, RTLD_NOW | RTLD_GLOBAL);
+			if (!handle)
+				ERR_RECORD("loader error: dlopen(%s): %s",
+					   fullpath, dlerror());
 		}
 		if (!handle) {
 			snprintf(fullpath, sizeof(fullpath), "%s/%s.so", dir,
 				 name);
 			handle = dlopen(fullpath, RTLD_NOW | RTLD_GLOBAL);
+			if (!handle)
+				ERR_RECORD("loader error: dlopen(%s): %s",
+					   fullpath, dlerror());
 		}
 	}
 	if (!handle) {
 		snprintf(fullpath, sizeof(fullpath), "%s", name);
 		handle = dlopen(fullpath, RTLD_NOW | RTLD_GLOBAL);
+		if (!handle)
+			ERR_RECORD("loader error: dlopen(%s): %s", fullpath,
+				   dlerror());
 	}
 	if (!handle) {
-		if (err)
-			snprintf(err, err_len,
-				 "loading module \"%s\" failed: %s", name,
-				 dlerror());
+		ERR_REPORT;
+		ERR_FREE;
 		return NULL;
 	}
 
+	/* previous dlopen() errors are no longer relevant */
+	ERR_FREE;
+
 	rtinfop = dlsym(handle, "frr_module");
 	if (!rtinfop) {
 		dlclose(handle);
-		if (err)
-			snprintf(err, err_len,
-				 "\"%s\" is not an FRR module: %s", name,
-				 dlerror());
+		ERR_RECORD("\"%s\" is not an FRR module: %s", name, dlerror());
+		ERR_REPORT;
+		ERR_FREE;
 		return NULL;
 	}
 	rtinfo = *rtinfop;
@@ -122,17 +183,13 @@ struct frrmod_runtime *frrmod_load(const char *spec, const char *dir, char *err,
 
 	if (rtinfo->finished_loading) {
 		dlclose(handle);
-		if (err)
-			snprintf(err, err_len, "module \"%s\" already loaded",
-				 name);
+		ERR_RECORD("module \"%s\" already loaded", name);
 		goto out_fail;
 	}
 
 	if (info->init && info->init()) {
 		dlclose(handle);
-		if (err)
-			snprintf(err, err_len,
-				 "module \"%s\" initialisation failed", name);
+		ERR_RECORD("module \"%s\" initialisation failed", name);
 		goto out_fail;
 	}
 
@@ -140,11 +197,14 @@ struct frrmod_runtime *frrmod_load(const char *spec, const char *dir, char *err,
 
 	*frrmod_last = rtinfo;
 	frrmod_last = &rtinfo->next;
+	ERR_FREE;
 	return rtinfo;
 
 out_fail:
 	XFREE(MTYPE_MODULE_LOADARGS, rtinfo->load_args);
 	XFREE(MTYPE_MODULE_LOADNAME, rtinfo->load_name);
+	ERR_REPORT;
+	ERR_FREE;
 	return NULL;
 }
 
diff --git a/lib/module.h b/lib/module.h
index 6275877cb3..ae1ca2f757 100644
--- a/lib/module.h
+++ b/lib/module.h
@@ -91,7 +91,9 @@ extern struct frrmod_runtime *frrmod_list;
 
 extern void frrmod_init(struct frrmod_runtime *modinfo);
 extern struct frrmod_runtime *frrmod_load(const char *spec, const char *dir,
-					  char *err, size_t err_len);
+					  void (*pFerrlog)(const void *,
+							   const char *),
+					  const void *pErrlogCookie);
 #if 0
 /* not implemented yet */
 extern void frrmod_unload(struct frrmod_runtime *module);