[DTrace-devel] [PATCH v2 3/4] EINTR: make safe under signal hits

[Eugene Loh]

There were my comments to v1 of this patch.  Plus, there are leading 
spaces in cases, where leading tabs should be used. Also...

On 11/1/23 11:31, Nick Alcock via DTrace-devel wrote:
> diff --git a/libcommon/dof_parser_host.c b/libcommon/dof_parser_host.c
> @@ -10,6 +10,7 @@
>   #include <stddef.h>
>   #include <stdlib.h>
>   #include <string.h>
> +#include <time.h>
>   #include <unistd.h>
>   
>   #include "dof_parser.h"
> @@ -88,14 +89,38 @@ dof_parser_host_read(int in, int timeout)
>   	 * only after that, both to make sure we don't underread and to make
>   	 * sure we don't *overread* and concatenate part of another message
>   	 * onto this one.
> +	 *
> +	 * Adjust the timeout whenever we are interrupted.  If we can't figure
> +	 * out the time, don't bother adjusting, but still read: a read taking
> +	 * longer than expected is better than no read at all.
>   	 */
>   	for (i = 0, sz = offsetof(dof_parsed_t, type); i < sz;) {
>   		size_t ret;
> +		struct timespec start, end;
> +		int no_adjustment = 0;
> +		long timeout_msec = timeout * 1000;
>   
> -		if ((ret = poll(&fd, 1, timeout * 1000)) <= 0)
> +		if (clock_gettime(CLOCK_REALTIME, &start) < 0)
> +			no_adjustment = 1;
> +
> +                while ((ret = poll(&fd, 1, timeout_msec)) <= 0 && errno == EINTR) {
> +
> +                        if (no_adjustment || clock_gettime(CLOCK_REALTIME, &end) < 0)
> +				continue; /* Abandon timeout adjustment */
> +
> +			timeout_msec -= ((end.tv_sec + ((unsigned long long) end.tv_nsec * NANOSEC)) -
> +					 (start.tv_sec + ((unsigned long long) start.tv_nsec * NANOSEC))) /
> +					MICROSEC;

Is that conversion correct?  secs + nsecs * 1000000000?

Also, while dividing by MICROSEC gives the right result, strictly 
speaking one maybe wants to divide by (NANOSEC/MILLISEC).  And to be 
stylistically consistent, use MILLISEC instead of the 1000.  (Or, just 
dispense with NANOSEC and MICROSEC and use 1000000000 and 1000000 instead.)

> +
> +			if (timeout_msec < 0)
> +				timeout_msec = 0;
> +		}