On Fri, Mar 28, 2025 at 12:36:25PM +0100, Yannick Le Pennec wrote:
On Thu, 2025-03-27 at 23:13 -0500, Lucas De Marchi wrote:On Mon, Mar 24, 2025 at 06:35:53PM +0100, Yannick Le Pennec wrote: > a6f9cd0 ("tools/rmmod: consistently use ERR logging facility") fixed > the split between syslog and stderr of various error message substrings > by calling the ERR macro instead of writing directly to stderr, but in > doing so also completely mangled the output because the ERR macro > decorates its arguments: > > $ rmmod iwlwifi > rmmod: ERROR: Module iwlwifi is in use by:rmmod: ERROR: iwlmvmrmmod: ERROR: > > And in syslog: > > $ rmmod -s iwlwifi > 2025-03-24T17:22:34.878318+01:00 mangolassi rmmod: ERROR: Module iwlwifi is in use by: > 2025-03-24T17:22:34.889145+01:00 mangolassi rmmod: ERROR: iwlmvm > 2025-03-24T17:22:34.889224+01:00 mangolassi rmmod: ERROR: > > This commit fixes that by building the holder names list with a strbuf > and then passes the whole thing at once to ERR.doesn't that mean the syslog version will only have 1 ERROR now?Yes indeed but I think that's the right behaviour. Prior to a6f9cd0 there was only 1 error going to syslog (because the rest of the line was sent to stderr erroneously). With a6f9cd0 N + 2 (with N = number of holders) error lines go to syslog, which I don't think is what was intended? After all the stderr message itself was always one line, and furthermore the lone ERR("\n") was odd.
oh... ok. I missed the fact that you dropped the newline and just added a space as sep. Applied, thanks. Lucas De Marchi
anyway, queued for CI at https://github.com/kmod-project/kmod/pull/328 thanks Lucas De Marchi > > Fixes: a6f9cd0 ("tools/rmmod: consistently use ERR logging facility") > Signed-off-by: Yannick Le Pennec <yannick.lepen...@live.fr> > --- > tools/rmmod.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/tools/rmmod.c b/tools/rmmod.c > index 962d850..61f2e00 100644 > --- a/tools/rmmod.c > +++ b/tools/rmmod.c > @@ -15,6 +15,7 @@ > #include <sys/types.h> > > #include <shared/macro.h> > +#include <shared/strbuf.h> > > #include <libkmod/libkmod.h> > > @@ -63,16 +64,18 @@ static int check_module_inuse(struct kmod_module *mod) > > holders = kmod_module_get_holders(mod); > if (holders != NULL) { > + DECLARE_STRBUF_WITH_STACK(buf, 128); > struct kmod_list *itr; > > - ERR("Module %s is in use by:", kmod_module_get_name(mod)); > - > kmod_list_foreach(itr, holders) { > struct kmod_module *hm = kmod_module_get_module(itr); > - ERR(" %s", kmod_module_get_name(hm)); > + strbuf_pushchar(&buf, ' '); > + strbuf_pushchars(&buf, kmod_module_get_name(hm)); > kmod_module_unref(hm); > } > - ERR("\n"); > + > + ERR("Module %s is in use by:%s\n", kmod_module_get_name(mod), > + strbuf_str(&buf)); > > kmod_module_unref_list(holders); > return -EBUSY; > -- > 2.49.0
