[PATCH] rfkill: allow user to exit gracefully from event watching loop

[PATCH] rfkill: allow user to exit gracefully from event watching loop

[Sami Kerola]

When SIGINT is raised by something else than an user running rfkill the
watch loop will be killed by that signal with appropriate exit code.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/rfkill.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/sys-utils/rfkill.c b/sys-utils/rfkill.c
index c9559ef48..d12942de0 100644
--- a/sys-utils/rfkill.c
+++ b/sys-utils/rfkill.c
@@ -27,6 +27,7 @@
 #include <sys/poll.h>
 #include <sys/syslog.h>
 #include <sys/time.h>
+#include <signal.h>
 
 #include "c.h"
 #include "closestream.h"
@@ -218,6 +219,21 @@ static int rfkill_read_event(int fd, struct rfkill_event *event)
 	return 0;
 }
 
+sig_atomic_t user_ctrl_c = 0;
+
+static void event_signal(int signo, siginfo_t *info,
+			 void *context __attribute__((__unused__)))
+{
+	if (!info->si_pid)
+		user_ctrl_c = 1;
+	else {
+		if (info->si_code == SI_USER)
+			warnx(_("uid: %d pid: %d sent signal %d, exiting"),
+			      info->si_uid, info->si_pid, info->si_signo);
+		signal(signo, SIG_DFL);
+		kill(getpid(), signo);
+	}
+}
 
 static int rfkill_event(void)
 {
@@ -226,6 +242,7 @@ static int rfkill_event(void)
 	char date_buf[ISO_8601_BUFSIZ];
 	struct pollfd p;
 	int fd, n;
+	struct sigaction sigact;
 
 	fd = rfkill_ro_open(0);
 	if (fd < 0)
@@ -235,13 +252,19 @@ static int rfkill_event(void)
 	p.fd = fd;
 	p.events = POLLIN | POLLHUP;
 
+	sigemptyset(&sigact.sa_mask);
+	sigact.sa_flags = SA_SIGINFO;
+	sigact.sa_sigaction = event_signal;
+	sigaction(SIGINT, &sigact, NULL);
+
 	/* interrupted by signal only */
-	while (1) {
+	while (!user_ctrl_c) {
 		int rc = 1;	/* recover-able error */
 
 		n = poll(&p, 1, -1);
 		if (n < 0) {
-			warn(_("failed to poll %s"), _PATH_DEV_RFKILL);
+			if (!user_ctrl_c)
+				warn(_("failed to poll %s"), _PATH_DEV_RFKILL);
 			goto failed;
 		}
 
@@ -267,7 +290,7 @@ static int rfkill_event(void)
 
 failed:
 	close(fd);
-	return -1;
+	return user_ctrl_c ? 0 : -1;
 }
 
 static const char *get_sys_attr(uint32_t idx, const char *attr)
-- 
2.15.0

Re: [PATCH] rfkill: allow user to exit gracefully from event watching loop

[Karel Zak]

On Tue, Nov 07, 2017 at 04:08:56PM +0000, Sami Kerola wrote:
> When SIGINT is raised by something else than an user running rfkill the
> watch loop will be killed by that signal with appropriate exit code.

It seems like over-engineering that introduces unnecessary code and
complex semantic. Why do you care about return code when you press
CTRL+C and why it should be different to another situations with
signals? I think we do not have such code in another utils.

> +static void event_signal(int signo, siginfo_t *info,
> +			 void *context __attribute__((__unused__)))
> +{
> +	if (!info->si_pid)
> +		user_ctrl_c = 1;
> +	else {
> +		if (info->si_code == SI_USER)
> +			warnx(_("uid: %d pid: %d sent signal %d, exiting"),
> +			      info->si_uid, info->si_pid, info->si_signo);

We should not use warn/err/printf in signals handlers.

> +		signal(signo, SIG_DFL);
> +		kill(getpid(), signo);
> +	}
> +}

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH] rfkill: allow user to exit gracefully from event watching loop

[Sami Kerola]

On 8 November 2017 at 11:31, Karel Zak <kzak@redhat.com> wrote:
> On Tue, Nov 07, 2017 at 04:08:56PM +0000, Sami Kerola wrote:
>> When SIGINT is raised by something else than an user running rfkill the
>> watch loop will be killed by that signal with appropriate exit code.
>
> It seems like over-engineering that introduces unnecessary code and
> complex semantic. Why do you care about return code when you press
> CTRL+C and why it should be different to another situations with
> signals? I think we do not have such code in another utils.

My thinking is distincting inbetween user oneself vs someone / something
else sent signal is worth knowing. This sort of user exit or killed could be
done in other tools as well, but I reading the reply I don't think distinction
is seen interesting enough. Lets call this as rejected proposal and be
done with this.

-- 
Sami Kerola
http://www.iki.fi/kerolasa/