diff mbox series

kdb: include header in signal handling code

Message ID 20230517125423.930967-1-arnd@kernel.org
State New
Headers
Series kdb: include header in signal handling code |

Commit Message

Arnd Bergmann May 17, 2023, 12:54 p.m. UTC 
  From: Arnd Bergmann <arnd@arndb.de>

kdb_send_sig() is defined in the signal code and called from kdb,
but the declaration is part of the kdb internal code.
Include this from signal.c as well to avoid the warning:

kernel/signal.c:4789:6: error: no previous prototype for 'kdb_send_sig' [-Werror=missing-prototypes]

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 kernel/signal.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Doug Anderson May 17, 2023, 3:43 p.m. UTC | #1 
Hi,

On Wed, May 17, 2023 at 5:54 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> kdb_send_sig() is defined in the signal code and called from kdb,
> but the declaration is part of the kdb internal code.
> Include this from signal.c as well to avoid the warning:
>
> kernel/signal.c:4789:6: error: no previous prototype for 'kdb_send_sig' [-Werror=missing-prototypes]
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  kernel/signal.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Kees Cook May 17, 2023, 6:56 p.m. UTC | #2 
On Wed, May 17, 2023 at 02:54:09PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> kdb_send_sig() is defined in the signal code and called from kdb,
> but the declaration is part of the kdb internal code.
> Include this from signal.c as well to avoid the warning:
> 
> kernel/signal.c:4789:6: error: no previous prototype for 'kdb_send_sig' [-Werror=missing-prototypes]
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Kees Cook <keescook@chromium.org>
Daniel Thompson June 30, 2023, 3:24 p.m. UTC | #3 
On Wed, May 17, 2023 at 02:54:09PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> kdb_send_sig() is defined in the signal code and called from kdb,
> but the declaration is part of the kdb internal code.
> Include this from signal.c as well to avoid the warning:
>
> kernel/signal.c:4789:6: error: no previous prototype for 'kdb_send_sig' [-Werror=missing-prototypes]
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Sorry to be so late with this feedback! I got as far as queuing this up
for merge before the penny dropped...

> ---
>  kernel/signal.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 8f6330f0e9ca..d38df14f71ac 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -4780,6 +4780,8 @@ void __init signals_init(void)
>
>  #ifdef CONFIG_KGDB_KDB
>  #include <linux/kdb.h>
> +#include "debug/kdb/kdb_private.h"
> +

Isn't is better to move the prototype for kdb_send_sig() into
linux/kdb.h instead?

That's what other kdb helpers spread across the kernel do
(kdb_walk_kallsyms() for example).


Daniel.
Arnd Bergmann June 30, 2023, 3:31 p.m. UTC | #4 
On Fri, Jun 30, 2023, at 17:24, Daniel Thompson wrote:
> On Wed, May 17, 2023 at 02:54:09PM +0200, Arnd Bergmann wrote:
>> diff --git a/kernel/signal.c b/kernel/signal.c
>> index 8f6330f0e9ca..d38df14f71ac 100644
>> --- a/kernel/signal.c
>> +++ b/kernel/signal.c
>> @@ -4780,6 +4780,8 @@ void __init signals_init(void)
>>
>>  #ifdef CONFIG_KGDB_KDB
>>  #include <linux/kdb.h>
>> +#include "debug/kdb/kdb_private.h"
>> +
>
> Isn't is better to move the prototype for kdb_send_sig() into
> linux/kdb.h instead?
>
> That's what other kdb helpers spread across the kernel do
> (kdb_walk_kallsyms() for example).

Right, that is probably better here. Not sure if it's worth
reworking the branch if you already merged it, the difference
seems rather minor.

       Arnd
Daniel Thompson June 30, 2023, 3:39 p.m. UTC | #5 
On Fri, Jun 30, 2023 at 05:31:01PM +0200, Arnd Bergmann wrote:
> On Fri, Jun 30, 2023, at 17:24, Daniel Thompson wrote:
> > On Wed, May 17, 2023 at 02:54:09PM +0200, Arnd Bergmann wrote:
> >> diff --git a/kernel/signal.c b/kernel/signal.c
> >> index 8f6330f0e9ca..d38df14f71ac 100644
> >> --- a/kernel/signal.c
> >> +++ b/kernel/signal.c
> >> @@ -4780,6 +4780,8 @@ void __init signals_init(void)
> >>
> >>  #ifdef CONFIG_KGDB_KDB
> >>  #include <linux/kdb.h>
> >> +#include "debug/kdb/kdb_private.h"
> >> +
> >
> > Isn't is better to move the prototype for kdb_send_sig() into
> > linux/kdb.h instead?
> >
> > That's what other kdb helpers spread across the kernel do
> > (kdb_walk_kallsyms() for example).
>
> Right, that is probably better here. Not sure if it's worth
> reworking the branch if you already merged it, the difference
> seems rather minor.

I figure it will take me as long to rework the branch as it will to
write the covering letter on the pull-request to explain why kgdb/kdb
is messing around in kernel/signal.c ;-) .


Daniel.
diff mbox series

Patch

diff --git a/kernel/signal.c b/kernel/signal.c
index 8f6330f0e9ca..d38df14f71ac 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -4780,6 +4780,8 @@  void __init signals_init(void)
 
 #ifdef CONFIG_KGDB_KDB
 #include <linux/kdb.h>
+#include "debug/kdb/kdb_private.h"
+
 /*
  * kdb_send_sig - Allows kdb to send signals without exposing
  * signal internals.  This function checks if the required locks are