Re: [B.A.T.M.A.N.] [PATCH] alfred: Externalized synchronization interval

[Simon Wunderlich <sw@simonwunderlich.de>]

[-- Attachment #1: Type: text/plain, Size: 1435 bytes --]

On Monday, August 15, 2016 2:59:05 PM CEST Jonathan Haws wrote:
> > > ALFRED_INTERVAL is now externalized via the -p option (synchronization
> > > period).  If specified as option, user supplied interval is used,
> > > otherwise the default interval of ALFRED_INTERVAL is used.
> > 
> > [...]
> > 
> > > --- a/server.c
> > > +++ b/server.c
> > > @@ -372,7 +372,7 @@ int alfred_server(struct globals *globals)
> > > 
> > > 
> > >  	while (1) {
> > >  	
> > >  		clock_gettime(CLOCK_MONOTONIC, &now);
> > > 
> > > -		now.tv_sec -= ALFRED_INTERVAL;
> > > +		time_diff(&now, &globals->sync_period, &now);
> > 
> > 
> > I believe this at least deserves a comment, or must be made more clear in
> > the 
 code.
> 
> 
> This change simply does exactly what the previous code did - subtracts
> the interval from the current time.  It utilizes the existing
> time_diff() routine to account for nanoseconds.
> 
> If you need me to resubmit with that language in the commit text or in a
> C comment, I can do that.
> 
> Thanks!

I was staring at your code and time_diff() which was implemented for an 
entirely different purpose, and concluded that you could be doing the right 
thing ... ;)

But yes, please re-submit with a comment explaining that, or restructure 
otherwise to make it more clear. time_diff() was written to get a difference 
of two timestamps, not to subtracts times from each other, and that confused 
me.

Thanks,
     Simon

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]