From 96025fd0e067acbc37970da77640254bfbbb2738 Mon Sep 17 00:00:00 2001
From: tb <tb@openbsd.org>
Date: Tue, 26 Mar 2024 00:24:11 +0000
Subject: [PATCH] Clean up conf's module_init()

Immediately error out when no name or value is passed instead of hiding
this in a a combination of ternary operator and strdup error check.
Use calloc(). Unindent some stupid, don't pretend this function can return
anything but -1 and 1, turn the whole thing into single exit and call the
now existing imodule_free() instead of handrolling it.

ok jsing
---
 lib/libcrypto/conf/conf_mod.c | 66 ++++++++++++++---------------------
 1 file changed, 27 insertions(+), 39 deletions(-)

diff --git a/lib/libcrypto/conf/conf_mod.c b/lib/libcrypto/conf/conf_mod.c
index 280d8edd201..85d74e6fb62 100644
--- a/lib/libcrypto/conf/conf_mod.c
+++ b/lib/libcrypto/conf/conf_mod.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: conf_mod.c,v 1.36 2024/03/20 22:11:07 tb Exp $ */
+/* $OpenBSD: conf_mod.c,v 1.37 2024/03/26 00:24:11 tb Exp $ */
 /* Written by Stephen Henson (steve@openssl.org) for the OpenSSL
  * project 2001.
  */
@@ -279,62 +279,50 @@ module_find(char *name)
 static int
 module_init(CONF_MODULE *mod, char *name, char *value, const CONF *cnf)
 {
-	int ret = 1;
-	int init_called = 0;
 	CONF_IMODULE *imod = NULL;
+	int need_finish = 0;
+	int ret = -1;
 
-	/* Otherwise add initialized module to list */
-	imod = malloc(sizeof(CONF_IMODULE));
-	if (!imod)
+	if (name == NULL || value == NULL)
+		goto err;
+
+	if ((imod = calloc(1, sizeof(*imod))) == NULL)
 		goto err;
 
 	imod->mod = mod;
-	imod->name = name ? strdup(name) : NULL;
-	imod->value = value ? strdup(value) : NULL;
-	imod->usr_data = NULL;
-
-	if (!imod->name || !imod->value)
-		goto memerr;
-
-	/* Try to initialize module */
-	if (mod->init) {
-		ret = mod->init(imod, cnf);
-		init_called = 1;
-		/* Error occurred, exit */
-		if (ret <= 0)
+
+	if ((imod->name = strdup(name)) == NULL)
+		goto err;
+	if ((imod->value = strdup(value)) == NULL)
+		goto err;
+
+	if (mod->init != NULL) {
+		need_finish = 1;
+		if (mod->init(imod, cnf) <= 0)
 			goto err;
 	}
 
-	if (initialized_modules == NULL) {
+	if (initialized_modules == NULL)
 		initialized_modules = sk_CONF_IMODULE_new_null();
-		if (!initialized_modules) {
-			CONFerror(ERR_R_MALLOC_FAILURE);
-			goto err;
-		}
-	}
+	if (initialized_modules == NULL)
+		goto err;
 
-	if (!sk_CONF_IMODULE_push(initialized_modules, imod)) {
-		CONFerror(ERR_R_MALLOC_FAILURE);
+	if (!sk_CONF_IMODULE_push(initialized_modules, imod))
 		goto err;
-	}
+	imod = NULL;
+	need_finish = 0;
 
 	mod->links++;
 
-	return ret;
+	ret = 1;
 
-err:
-	/* We've started the module so we'd better finish it */
-	if (mod->finish && init_called)
+ err:
+	if (need_finish && mod->finish != NULL)
 		mod->finish(imod);
 
-memerr:
-	if (imod) {
-		free(imod->name);
-		free(imod->value);
-		free(imod);
-	}
+	imodule_free(imod);
 
-	return -1;
+	return ret;
 }
 
 /* Unload any dynamic modules that have a link count of zero:
-- 
2.20.1