[lttng-dev] [PATCH lttng-tools] Fix: nsec diff can be negative leading to expeditive connection timeout

[Simon Marchi]

On 2018-11-08 17:08, Jonathan Rajotte wrote:
> The nanoseconds part of the timespec struct time_a is not always
> bigger than time_b since it wrap around each seconds.
> 
> Use the absolute value of the nanosecond difference to perform
> unsigned long operation.
> 
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
> ---
>  src/common/sessiond-comm/inet.c  | 2 +-
>  src/common/sessiond-comm/inet6.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/common/sessiond-comm/inet.c 
> b/src/common/sessiond-comm/inet.c
> index e0b3e7a96..cb6f45357 100644
> --- a/src/common/sessiond-comm/inet.c
> +++ b/src/common/sessiond-comm/inet.c
> @@ -124,7 +124,7 @@ unsigned long time_diff_ms(struct timespec *time_a,
>  	unsigned long result_ms;
> 
>  	sec_diff = time_a->tv_sec - time_b->tv_sec;
> -	nsec_diff = time_a->tv_nsec - time_b->tv_nsec;
> +	nsec_diff = labs(time_a->tv_nsec - time_b->tv_nsec);
> 
>  	result_ms = sec_diff * MSEC_PER_SEC;
>  	result_ms += nsec_diff / NSEC_PER_MSEC;
> diff --git a/src/common/sessiond-comm/inet6.c 
> b/src/common/sessiond-comm/inet6.c
> index dfb5fc5d1..b73802d48 100644
> --- a/src/common/sessiond-comm/inet6.c
> +++ b/src/common/sessiond-comm/inet6.c
> @@ -122,7 +122,7 @@ unsigned long time_diff_ms(struct timespec *time_a,
>  	unsigned long result_ms;
> 
>  	sec_diff = time_a->tv_sec - time_b->tv_sec;
> -	nsec_diff = time_a->tv_nsec - time_b->tv_nsec;
> +	nsec_diff = labs(time_a->tv_nsec - time_b->tv_nsec);
> 
>  	result_ms = sec_diff * MSEC_PER_SEC;
>  	result_ms += nsec_diff / NSEC_PER_MSEC;

That doesn't look right either.  With

time_a = 2.9 s
time_b = 3.1 s

it will result in

sec_diff = 1 s
nsec_diff = 0.8 s

Added, the result will be 1.8 seconds, when actually we expect 0.2 s.  
You probably have no choice but do the "borrowing" by hand, like we did 
when learning subtraction in elementary school :).  Maybe this deserves 
some unit tests, since it's apparently easy to get it wrong.

Simon