diff mbox series

[v2,24/39] gdbserver: catch ctrl-C to clean up host state

Message ID 20220420065013.222816-25-npiggin@gmail.com
State New
Headers show
Series gdbserver multi-threaded debugging and POWER9/10 support | expand

Commit Message

Nicholas Piggin April 20, 2022, 6:49 a.m. UTC 
This allows gdbserver to clean up the host state gracefully after
a SIGINT. Later patches will add more host state cleanup.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 src/pdbgproxy.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

Comments

Joel Stanley May 3, 2022, 7:13 a.m. UTC | #1 
On Wed, 20 Apr 2022 at 06:51, Nicholas Piggin <npiggin@gmail.com> wrote:
>
> This allows gdbserver to clean up the host state gracefully after
> a SIGINT. Later patches will add more host state cleanup.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  src/pdbgproxy.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/src/pdbgproxy.c b/src/pdbgproxy.c
> index 4572c689..ef8aa479 100644
> --- a/src/pdbgproxy.c
> +++ b/src/pdbgproxy.c
> @@ -4,6 +4,7 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <signal.h>
>  #include <fcntl.h>
>  #include <unistd.h>
>  #include <sys/types.h>
> @@ -640,12 +641,28 @@ static command_cb callbacks[LAST_CMD + 1] = {
>         detach,
>         NULL};
>
> -int gdbserver_start(struct pdbg_target *thread, struct pdbg_target *adu, uint16_t port)
> +static volatile bool gdbserver_running = true;

volatile! Is this how we userspace?

Reviewed-by: Joel Stanley <joel@jms.id.au>

> +
> +static void SIGINT_handler(int sig)
> +{
> +       gdbserver_running = false;
> +}
> +
> +static int gdbserver_start(struct pdbg_target *thread, struct pdbg_target *adu, uint16_t port)
>  {
>         int sock, i;
> +       struct sigaction sa;
>         struct sockaddr_in name;
>         fd_set active_fd_set, read_fd_set;
>
> +       memset(&sa, 0, sizeof(sa));
> +       sa.sa_handler = SIGINT_handler;
> +       sa.sa_flags = SA_RESETHAND | SA_NODEFER;
> +       if (sigaction(SIGINT, &sa, NULL) == -1) {
> +               perror("sigaction");
> +               return -1;
> +       }
> +
>         parser_init(callbacks);
>         thread_target = thread;
>         adu_target = adu;
> @@ -674,7 +691,7 @@ int gdbserver_start(struct pdbg_target *thread, struct pdbg_target *adu, uint16_
>         FD_ZERO(&active_fd_set);
>         FD_SET(sock, &active_fd_set);
>
> -       while (1) {
> +       while (gdbserver_running) {
>                 read_fd_set = active_fd_set;
>                 timeout.tv_sec = 0;
>                 timeout.tv_usec = poll_interval;
> @@ -725,6 +742,8 @@ int gdbserver_start(struct pdbg_target *thread, struct pdbg_target *adu, uint16_
>                 poll();
>         }
>
> +       printf("gdbserver: got ctrl-C, cleaning up (second ctrl-C to kill immediately).\n");
> +
>         return 1;
>  }
>
> @@ -776,6 +795,7 @@ static int gdbserver(uint16_t port)
>         }
>
>         gdbserver_start(thread, adu, port);
> +
>         return 0;
>  }
>  #else
> --
> 2.35.1
>
> --
> Pdbg mailing list
> Pdbg@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/pdbg
Nicholas Piggin May 10, 2022, 9:53 a.m. UTC | #2 
Excerpts from Joel Stanley's message of May 3, 2022 5:13 pm:
> On Wed, 20 Apr 2022 at 06:51, Nicholas Piggin <npiggin@gmail.com> wrote:
>>
>> This allows gdbserver to clean up the host state gracefully after
>> a SIGINT. Later patches will add more host state cleanup.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>  src/pdbgproxy.c | 24 ++++++++++++++++++++++--
>>  1 file changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/pdbgproxy.c b/src/pdbgproxy.c
>> index 4572c689..ef8aa479 100644
>> --- a/src/pdbgproxy.c
>> +++ b/src/pdbgproxy.c
>> @@ -4,6 +4,7 @@
>>  #include <stdio.h>
>>  #include <stdlib.h>
>>  #include <string.h>
>> +#include <signal.h>
>>  #include <fcntl.h>
>>  #include <unistd.h>
>>  #include <sys/types.h>
>> @@ -640,12 +641,28 @@ static command_cb callbacks[LAST_CMD + 1] = {
>>         detach,
>>         NULL};
>>
>> -int gdbserver_start(struct pdbg_target *thread, struct pdbg_target *adu, uint16_t port)
>> +static volatile bool gdbserver_running = true;
> 
> volatile! Is this how we userspace?

I'm rusty but I think this is one of the few occasions you can use
volatile without either being wrong or having a compiler writer get
angry at you.

Thanks,
Nick

> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> 
>> +
>> +static void SIGINT_handler(int sig)
>> +{
>> +       gdbserver_running = false;
>> +}
>> +
>> +static int gdbserver_start(struct pdbg_target *thread, struct pdbg_target *adu, uint16_t port)
>>  {
>>         int sock, i;
>> +       struct sigaction sa;
>>         struct sockaddr_in name;
>>         fd_set active_fd_set, read_fd_set;
>>
>> +       memset(&sa, 0, sizeof(sa));
>> +       sa.sa_handler = SIGINT_handler;
>> +       sa.sa_flags = SA_RESETHAND | SA_NODEFER;
>> +       if (sigaction(SIGINT, &sa, NULL) == -1) {
>> +               perror("sigaction");
>> +               return -1;
>> +       }
>> +
>>         parser_init(callbacks);
>>         thread_target = thread;
>>         adu_target = adu;
>> @@ -674,7 +691,7 @@ int gdbserver_start(struct pdbg_target *thread, struct pdbg_target *adu, uint16_
>>         FD_ZERO(&active_fd_set);
>>         FD_SET(sock, &active_fd_set);
>>
>> -       while (1) {
>> +       while (gdbserver_running) {
>>                 read_fd_set = active_fd_set;
>>                 timeout.tv_sec = 0;
>>                 timeout.tv_usec = poll_interval;
>> @@ -725,6 +742,8 @@ int gdbserver_start(struct pdbg_target *thread, struct pdbg_target *adu, uint16_
>>                 poll();
>>         }
>>
>> +       printf("gdbserver: got ctrl-C, cleaning up (second ctrl-C to kill immediately).\n");
>> +
>>         return 1;
>>  }
>>
>> @@ -776,6 +795,7 @@ static int gdbserver(uint16_t port)
>>         }
>>
>>         gdbserver_start(thread, adu, port);
>> +
>>         return 0;
>>  }
>>  #else
>> --
>> 2.35.1
>>
>> --
>> Pdbg mailing list
>> Pdbg@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/pdbg
>
diff mbox series

Patch

diff --git a/src/pdbgproxy.c b/src/pdbgproxy.c
index 4572c689..ef8aa479 100644
--- a/src/pdbgproxy.c
+++ b/src/pdbgproxy.c
@@ -4,6 +4,7 @@ 
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <signal.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <sys/types.h>
@@ -640,12 +641,28 @@  static command_cb callbacks[LAST_CMD + 1] = {
 	detach,
 	NULL};
 
-int gdbserver_start(struct pdbg_target *thread, struct pdbg_target *adu, uint16_t port)
+static volatile bool gdbserver_running = true;
+
+static void SIGINT_handler(int sig)
+{
+	gdbserver_running = false;
+}
+
+static int gdbserver_start(struct pdbg_target *thread, struct pdbg_target *adu, uint16_t port)
 {
 	int sock, i;
+	struct sigaction sa;
 	struct sockaddr_in name;
 	fd_set active_fd_set, read_fd_set;
 
+	memset(&sa, 0, sizeof(sa));
+	sa.sa_handler = SIGINT_handler;
+	sa.sa_flags = SA_RESETHAND | SA_NODEFER;
+	if (sigaction(SIGINT, &sa, NULL) == -1) {
+		perror("sigaction");
+		return -1;
+	}
+
 	parser_init(callbacks);
 	thread_target = thread;
 	adu_target = adu;
@@ -674,7 +691,7 @@  int gdbserver_start(struct pdbg_target *thread, struct pdbg_target *adu, uint16_
 	FD_ZERO(&active_fd_set);
 	FD_SET(sock, &active_fd_set);
 
-	while (1) {
+	while (gdbserver_running) {
 		read_fd_set = active_fd_set;
 		timeout.tv_sec = 0;
 		timeout.tv_usec = poll_interval;
@@ -725,6 +742,8 @@  int gdbserver_start(struct pdbg_target *thread, struct pdbg_target *adu, uint16_
 		poll();
 	}
 
+	printf("gdbserver: got ctrl-C, cleaning up (second ctrl-C to kill immediately).\n");
+
 	return 1;
 }
 
@@ -776,6 +795,7 @@  static int gdbserver(uint16_t port)
 	}
 
 	gdbserver_start(thread, adu, port);
+
 	return 0;
 }
 #else