[BusyBox] rdate: Use read and not safe_read [PATCH]

[Rainer Weikusat]

Shaun Jackman <sjackman at gmail.com> writes:
> I'm not sure why safe_read was used, since the intention was for the
> alarm to interrupt the read, which safe_read prevents.

Because the signal handler aborts the program
('bb_error_message_and_die'). This is an error, because the function
is not async-signal-safe, but that's another issue.

> diff -ur busybox-1.00.orig/util-linux/rdate.c busybox-1.00/util-linux/rdate.c
> --- busybox-1.00.orig/util-linux/rdate.c	2004-01-18 10:18:33.000000000 -0800
> +++ busybox-1.00/util-linux/rdate.c	2005-03-17 11:28:50.000000000 -0800
> @@ -54,13 +54,15 @@
>  	s_in.sin_port = bb_lookup_port("time", "tcp", 37);
>  
>  	/* Add a timeout for dead or non accessable servers */
> -	alarm(10);
> +	alarm(5);
>  	signal(SIGALRM, socket_timeout);
>  
>  	fd = xconnect(&s_in);
>  
> -	if (safe_read(fd, (void *)&nett, 4) != 4)    /* read time from server */
> +	if (read(fd, (void *)&nett, 4) != 4)    /* read time from server */
>  		bb_error_msg_and_die("%s did not send the complete time", host);
> +	/* Cancel the timeout. */
> +	alarm(0);
>

And if the signal handler would not cause the program to exit, this
would be seriously broken because of various races which could all
lead to lost wakeups. One by one:

	1. You set the signal handler after starting the alarm. This
           means to alarm could go off before your handler is
           installed, causing a program abort.

	2. The alarm might go off before the connect in xconnect is
           reached, resulting in a normal blocking connect.

        3. The same is true for the 'read'.

The only way to make these kind of forced timeout reliable is by
longjmping (or exiting, as the original code does) from the handler.
Roughly, this would look as follows:

         static jmp_buf timeout_jump;

         static void alrm_handler(int signo)
         {
         	(void)signo;	/* keep gcc happy */
                longjmp(timeout_jump, 1);
         }

         [...]

         	if (setjmp(timeout_jmp)) {
                	/* handle timeout */
                }

                signal(SIGALRM, alrm_handler);
                alarm(timeout)

                /*
                  here goes the 'productive' code
                */