On Sun, May 26, 2019 at 11:25:12AM +0200, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx> The documentation says that kmod_module_probe_insert_module() will return >0 if "stopped by a reason given in @flags" but it returns a negative value if KMOD_PROBE_FAIL_ON_LOADED flag is passed and the relevant module is already loaded. Fix the error path by using a positive error value where required. Signed-off-by: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx> --- libkmod/libkmod-module.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c index bffe715..a3afaba 100644 --- a/libkmod/libkmod-module.c +++ b/libkmod/libkmod-module.c @@ -1253,7 +1253,7 @@ KMOD_EXPORT int kmod_module_probe_insert_module(struct kmod_module *mod, if (!(flags & KMOD_PROBE_IGNORE_LOADED) && module_is_inkernel(mod)) { if (flags & KMOD_PROBE_FAIL_ON_LOADED) - return -EEXIST; + return EEXIST; else return 0; } @@ -1304,7 +1304,7 @@ KMOD_EXPORT int kmod_module_probe_insert_module(struct kmod_module *mod, && module_is_inkernel(m)) { DBG(mod->ctx, "Ignoring module '%s': already loaded\n", m->name); - err = -EEXIST; + err = EEXIST;
this would break the ABI. modprobe for instance, that is in the same repository would be broken by it: kmod_list_foreach() { ... if (err >= 0) /* ignore flag return values such as a mod being blacklisted */ err = 0; else { switch (err) { case -EEXIST: ... } I think we need to say the bug is actually in the bad documentation - my bad actually, on commit b1a51256a9ed ("libkmod-module: probe: change insertion to cover more use cases"). If you look at the definition of 'enum kmod_probe' you will see that not all flags are returned as positive values - the only values that have this behavior are the ones related to blacklist. Could you rather patch the documentation? thanks Lucas De Marchi
goto finish_module; } @@ -1340,14 +1340,14 @@ finish_module: * been loaded between the check and the moment we try to * insert it. */ - if (err == -EEXIST && m == mod && + if (err == EEXIST && m == mod && (flags & KMOD_PROBE_FAIL_ON_LOADED)) break; /* * Ignore errors from softdeps */ - if (err == -EEXIST || !m->required) + if (err == EEXIST || !m->required) err = 0; else if (err < 0) -- 2.21.0