[PATCH] The NTP client/server applet

[Adam Tkac]

On Sun, Nov 22, 2009 at 03:53:10AM +0100, Denys Vlasenko wrote:
> On Saturday 21 November 2009 18:26, Adam Tkac wrote:
> > Hi all,
> > 
> > I just finished the NTP client/server applet. Would it be possible to
> > merge it to the main repository, please?
> 
> I did some changes - see the attached patch. Some
> of these simplififications are quite obvious,
> please do similar ones next time yourself.

Thanks for the review & acceptance.

> I did a VERY limited testing and found one bug. Fix
> (on top of attached):
> 
> -       bb_signals(SIGTERM|SIGINT, record_signo);
> -       bb_signals(SIGPIPE|SIGHUP, SIG_IGN);
> +       bb_signals((1 << SIGTERM) | (1 << SIGINT), record_signo);
> +       bb_signals((1 << SIGPIPE) | (1 << SIGHUP), SIG_IGN);

Ah, so this was the reason why I wasn't able to terminate ntpd via
"CTRL+C" :)

> I also changed the options to make them resemble upstream ntpd:

As I wrote in another post in this thread implementation is based on
OpenNTPD, not reference NTP. Due this reason I used OpenNTPd
parameters. But you are right that reference NTP parameters are widely
known.

> The applet definitely needs more testing.

Right you are, I'm going to perform more testing.

I agree with all your changes except one piece below.

Admins usually use "NTP pool" as the main time source (pool.ntp.org).
With your patch only one server is used. I know the old version of
add_peers consumes more space (+241B) but on the other hand it makes
ntpd more accurate.

Regards, Adam

>  static void
>  add_peers(const char *s)
>  {
> -	struct addrinfo		 hints, *res0, *res;
j> -	int			 error;
> -	ntp_peer_t		*p;
> -
> -	memset(&hints, 0, sizeof(hints));
> -	hints.ai_family = PF_UNSPEC;
> -	hints.ai_socktype = SOCK_DGRAM; /* DUMMY */
> -	error = getaddrinfo(s, NULL, &hints, &res0);
> -	if (error)
> -		bb_error_msg_and_die("DNS resolution failed for "
> -				     "peer %s", optarg);
> -
> -	for (res = res0; res != NULL; res = res->ai_next) {
> -		if (res->ai_family != AF_INET
> -#if ENABLE_FEATURE_IPV6
> -		    && res->ai_family != AF_INET6
> -#endif
> -		   )
> -			continue;
> -
> -		p = xzalloc(sizeof(*p));
> -		p->lsa = xzalloc(sizeof(*p->lsa));
> -		p->lsa->len =
> -#if ENABLE_FEATURE_IPV6
> -			res->ai_family == AF_INET6 ?
> -				sizeof(struct sockaddr_in6) :
> -#endif
> -				sizeof(struct sockaddr_in);
> -
> -		memcpy(&p->lsa->u.sa, res->ai_addr, p->lsa->len);
> -		p->lsa->u.sa.sa_family = res->ai_family;
> -		set_nport(p->lsa, htons(123));
> +	ntp_peer_t *p;
>  
> -		p->query.fd = -1;
> -		p->query.msg.status = MODE_CLIENT | (NTP_VERSION << 3);
> +	p = xzalloc(sizeof(*p));
> +//TODO: big ntpd uses all IPs, not just 1st, do we need to mimic that?
> +	p->lsa = xhost2sockaddr(s, 123);
> +	p->query.fd = -1;
> +	p->query.msg.status = MODE_CLIENT | (NTP_VERSION << 3);
> +	if (STATE_NONE != 0)
>  		p->state = STATE_NONE;
> -		p->trustlevel = TRUSTLEVEL_PATHETIC;
> -		p->query.fd = -1;
> -		set_next(p, 0);
> +	p->trustlevel = TRUSTLEVEL_PATHETIC;
> +	p->query.fd = -1;
> +	set_next(p, 0);
>  
> -		llist_add_to(&G->ntp_peers, p);
> -		G->peer_cnt++;
> -	}
> -	freeaddrinfo(res0);
> +	llist_add_to(&G->ntp_peers, p);
> +	G->peer_cnt++;
>  }
>  
>  static double

-- 
Adam Tkac