On Mon, Nov 08, 2021 at 11:19:24AM +0100, Borislav Petkov wrote:
From: Borislav Petkov <redacted>
Hi all,
this is a huge patchset for something which is really trivial - it
changes the notifier registration routines to return an error value
if a notifier callback is already present on the respective list of
callbacks. For more details scroll to the last patch.
Everything before it is converting the callers to check the return value
of the registration routines and issue a warning, instead of the WARN()
notifier_chain_register() does now.
What reason is there for moving the check into the callers? It seems
like pointless churn. Why not add the error return code, change the
WARN to pr_warn, and leave the callers as they are? Wouldn't that end
up having exactly the same effect?
For that matter, what sort of remedial action can a caller take if the
return code is -EEXIST? Is there any point in forcing callers to check
the return code if they can't do anything about it?
Before the last patch has been applied, though, that checking is a
NOP which would make the application of those patches trivial - every
maintainer can pick a patch at her/his discretion - only the last one
enables the build warnings and that one will be queued only after the
preceding patches have all been merged so that there are no build
warnings.
Why should there be _any_ build warnings? The real problem occurs when
a notifier callback is added twice, not when a caller fails to check the
return code. Double-registration is not the sort of thing that can be
detected at build time.
Alan Stern
Due to the sheer volume of the patches, I have addressed the respective
patch and the last one, which enables the warning, with addressees for
each maintained area so as not to spam people unnecessarily.
If people prefer I carry some through tip, instead, I'll gladly do so -
your call.
And, if you think the warning messages need to be more precise, feel
free to adjust them before committing.
Thanks!