From dcb885b5f6e64d2b1714d8553fd12404f67c20e5 Mon Sep 17 00:00:00 2001 From: Vladimír Čunát Date: Fri, 20 Dec 2024 13:27:41 +0100 Subject: kr_module_load(): avoid calling deinit() on errors If the module's init() failed or wasn't even called, I see it as an API error to call deinit(). When init() fails, it should take care of cleanup itself. --- lib/module.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'lib') diff --git a/lib/module.c b/lib/module.c index 79219d3a..2427c7b6 100644 --- a/lib/module.c +++ b/lib/module.c @@ -123,6 +123,8 @@ int kr_module_load(struct kr_module *module, const char *name, const char *path) ret = module->init(module); } if (ret != 0) { + /* Avoid calling deinit() as init() wasn't called or failed. */ + module->deinit = NULL; kr_module_unload(module); } -- cgit v1.2.3 From efbb03116feb37736f5dad3cd7d6a80c44b1e478 Mon Sep 17 00:00:00 2001 From: Vladimír Čunát Date: Fri, 20 Dec 2024 13:27:41 +0100 Subject: kr_module_load(): don't pass the_engine in module->data It was a confusing way of repurposing the field. My comment from over five years ago (129002fc0d) said that some external C modules might be relying on this. But that certainly sounds moot nowadays. To get more confidence, I rechecked all kr_module::data references (as found by libclang). --- daemon/engine.c | 2 -- lib/module.c | 4 +--- lib/module.h | 2 +- 3 files changed, 2 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/daemon/engine.c b/daemon/engine.c index 509915df..a0da529b 100644 --- a/daemon/engine.c +++ b/daemon/engine.c @@ -741,8 +741,6 @@ int engine_register(const char *name, const char *precedence, const char* ref) if (!module) { return kr_error(ENOMEM); } - module->data = the_engine; /*< some outside modules may still use this value */ - int ret = kr_module_load(module, name, LIBDIR "/kres_modules"); if (ret == 0) { /* We have a C module, loaded and init() was called. diff --git a/lib/module.c b/lib/module.c index 2427c7b6..df9a1f5e 100644 --- a/lib/module.c +++ b/lib/module.c @@ -103,10 +103,8 @@ int kr_module_load(struct kr_module *module, const char *name, const char *path) return kr_error(EINVAL); } - /* Initialize, keep userdata */ - void *data = module->data; + /* Initialize */ memset(module, 0, sizeof(struct kr_module)); - module->data = data; module->name = strdup(name); if (module->name == NULL) { return kr_error(ENOMEM); diff --git a/lib/module.h b/lib/module.h index 507b2df1..4dd5a490 100644 --- a/lib/module.h +++ b/lib/module.h @@ -86,7 +86,7 @@ struct kr_prop { /** * Load a C module instance into memory. And call its init(). * - * @param module module structure. Will be overwritten except for ->data on success. + * @param module module structure. Will be overwritten. * @param name module name * @param path module search path * @return 0 or an error -- cgit v1.2.3