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

[Jonathan Rajotte-Julien]

You are right...Might have been looking to long at logs all day.

Thanks for pointing it out.

Will send v2 shortly.

On Thu, Nov 08, 2018 at 05:26:39PM -0500, Simon Marchi wrote:
> On 2018-11-08 17:17, Simon Marchi wrote:
> > 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.
> 
> Actually, see here for an example:
> 
> https://www.gnu.org/software/libc/manual/html_node/Elapsed-Time.html
> 
> Simon

-- 
Jonathan Rajotte-Julien
EfficiOS