Re: [hackers] [PATCH] [ubase] dd: Use sigaction(2) to obviate select(2)

[Eric Pruitt]

On Sun, Sep 10, 2017 at 11:55:53AM +0100, Richard Ipsum wrote:
> This isn't a comment on this patch exactly, since this call exists in
> the existing handler, but unless I'm missing something about the way
> you're doing this it isn't safe to call fprintf in a signal handler
> since your signal may be interrupting an fprintf call itself, and
> since the data structures for stdio aren't reentrant, things will get
> messy.

I personally don't think there's any benefit in displaying the message
at all. I'll post a revised patch since moving the warning is trivial,
and it'll give me a chance to fix my botched commit message.

Eric

Re: [hackers] [PATCH] [ubase] dd: Use sigaction(2) to obviate select(2)

[Richard Ipsum]

On Sat, Sep 09, 2017 at 11:06:15PM -0700, Eric Pruitt wrote:
> By setting the SIGINT handler using sigaction(2), automatic retries of
> the splice(2) call can be disabled by not setting SA_RESTART. This makes
> it possible to use Ctrl+C even if standard input is a terminal.
> ---
[snip]
> +static void
> +sig_int(int unused_1, siginfo_t *unused_2, void *unused_3)
> +{
> + (void) unused_1;
> + (void) unused_2;
> + (void) unused_3;
> + fprintf(stderr, "SIGINT! Aborting ...\n");

This isn't a comment on this patch exactly, since this call
exists in the existing handler, but unless I'm missing something about
the way you're doing this it isn't safe to call fprintf in a signal
handler since your signal may be interrupting an fprintf call itself,
and since the data structures for stdio aren't reentrant, things will
get messy.

Richard

[hackers] [PATCH] [ubase] dd: Use sigaction(2) to obviate select(2)

[Eric Pruitt]

By setting the SIGINT handler using sigaction(2), automatic retries of
the splice(2) call can be disabled by not setting SA_RESTART. This makes
it possible to use Ctrl+C even if standard input is a terminal.
---
 dd.c | 58 +-
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git dd.c dd.c
index cc05d40..4011045 100644
--- dd.c
+++ dd.c
@@ -8,7 +8,6 @@
  */
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -38,6 +37,16 @@ struct dd_config {
 
 static int sigint = 0;
 
+static void
+sig_int(int unused_1, siginfo_t *unused_2, void *unused_3)
+{
+   (void) unused_1;
+   (void) unused_2;
+   (void) unused_3;
+   fprintf(stderr, "SIGINT! Aborting ...\n");
+   sigint = 1;
+}
+
 static int
 prepare_copy(struct dd_config *ddc, int *ifd, int *ofd)
 {
@@ -147,7 +156,6 @@ copy_splice(struct dd_config *ddc)
int ifd, ofd, p[2] = {-1, -1};
ssize_t r = 0;
size_t n = 0;
-   fd_set rfd, wfd;
 
if (prepare_copy(ddc, &ifd, &ofd) < 0)
return -1;
@@ -165,26 +173,19 @@ copy_splice(struct dd_config *ddc)
 #endif
n = ddc->bs;
for (;ddc->b_out != ddc->count && !sigint;) {
-   FD_ZERO(&rfd);
-   FD_ZERO(&wfd);
-   FD_SET(ifd, &rfd);
-   FD_SET(ofd, &wfd);
-   r = select(ifd > ofd ? ifd + 1 : ofd + 1, &rfd, &wfd, NULL, 
NULL);
if (r < 0)
break;
-   if (FD_ISSET(ifd, &rfd) == 1 && FD_ISSET(ofd, &wfd) == 1) {
-   if (n > ddc->count - ddc->b_out)
-   n = ddc->count - ddc->b_out;
-   r = splice(ifd, NULL, p[1], NULL, n, SPLICE_F_MORE);
-   if (r <= 0)
-   break;
-   ++ddc->rec_in;
-   r = splice(p[0], NULL, ofd, NULL, r, SPLICE_F_MORE);
-   if (r <= 0)
-   break;
-   ddc->b_out += r;
-   ++ddc->rec_out;
-   }
+   if (n > ddc->count - ddc->b_out)
+   n = ddc->count - ddc->b_out;
+   r = splice(ifd, NULL, p[1], NULL, n, SPLICE_F_MORE);
+   if (r <= 0)
+   break;
+   ++ddc->rec_in;
+   r = splice(p[0], NULL, ofd, NULL, r, SPLICE_F_MORE);
+   if (r <= 0)
+   break;
+   ddc->b_out += r;
+   ++ddc->rec_out;
}
close(ifd);
close(ofd);
@@ -227,14 +228,6 @@ print_stat(const struct dd_config *ddc)
 }
 
 static void
-sig_int(int unused)
-{
-   (void) unused;
-   fprintf(stderr, "SIGINT! Aborting ...\n");
-   sigint = 1;
-}
-
-static void
 usage(void)
 {
eprintf("usage: %s [-h] [if=infile] [of=outfile] [bs[=N]] [seek=N] "
@@ -248,6 +241,7 @@ main(int argc, char *argv[])
int i = 0;
char buf[1024];
struct dd_config config;
+   struct sigaction sa;
 
argv0 = argv[0];
memset(&config, 0, sizeof(config));
@@ -286,7 +280,13 @@ main(int argc, char *argv[])
}
 
signal(SIGPIPE, SIG_IGN);
-   signal(SIGINT, sig_int);
+
+   sa.sa_flags = SA_SIGINFO;
+   sigemptyset(&sa.sa_mask);
+   sa.sa_sigaction = sig_int;
+
+   if (sigaction(SIGINT, &sa, NULL) == -1)
+   weprintf("sigaction");
 
if (copy(&config) < 0)
weprintf("copy:");
-- 
2.11.0