Messages in this thread | | | From | Marco Elver <> | Date | Thu, 6 May 2021 18:11:40 +0200 | Subject | Re: [PATCH] checkpatch: warn about direct use of send_sig_info and force_sig_info |
| |
On Thu, 6 May 2021 at 18:02, Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > On Thu, May 6, 2021 at 5:02 PM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote: > > On Thu, May 6, 2021 at 6:59 PM Marco Elver <elver@google.com> wrote: > > > > > > Setting up siginfo and using send_sig_info() and force_sig_info() > > > directly is discouraged. Instead, new code wanting to generate signals > > > should use the appropriate helper specific to the signal. > > > > > > Eric mentioned that he'd like to make these static at some point, but > > > until that can happen, let's try to avoid introducing new users of them. > > > > > > Cc: Eric W. Biederman <ebiederm@xmission.com> > > > Signed-off-by: Marco Elver <elver@google.com> > > > --- > > > Eric, > > > > > > While siginfo doesn't need changing often, when it does, it's quite the > > > adventure. We now have the various static asserts. The other thing is > > > usage of {send,force}_sig_info. > > > > > > I think the best option right now is to teach checkpatch.pl about it > > > until they become static. > > > > > > Fyi, I noticed one such new user here: > > > https://lkml.kernel.org/r/20210421024826.13529-1-wangjunqiang@iscas.ac.cn > > > > > > Thanks, > > > -- Marco > > > --- > > > scripts/checkpatch.pl | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > > index ccb412a74725..3a86aafc3bcd 100755 > > > --- a/scripts/checkpatch.pl > > > +++ b/scripts/checkpatch.pl > > > @@ -7153,6 +7153,12 @@ sub process { > > > "Where possible, use lockdep_assert_held instead of assertions based on spin_is_locked\n" . $herecurr); > > > } > > > > > > +# check for direct use of send_sig_info(), force_sig_info() > > > + if ($line =~ /\b((force|send)_sig_info)\(/) { > > > > I think this might be a little better as: > > if ($line =~ /\b((?:force|send)_sig_info)\(/) { > > > > Otherwise it's good as it is. > > Tested-by: Dwaipayan Ray <dwaipayanray1@gmail.com> > > > > Dwaipayan, do you want to also document this new rule on the > checkpatch documentation? > Marco, maybe you can assist us here with some pointer (lore.kernel.org > link) to the original discussion you had.
It started somewhere here: https://lkml.kernel.org/r/m17dkjttpj.fsf@fess.ebiederm.org
Eric has the full history here -- if I missed something, hopefully he'll nack or ack.
Thanks, -- Marco
| |