Improve ntpd offset handling. Call the index of the offset loops

"shift" consistently.  Merge the two offset loops in client_update()
into one.  Use a simple assignment for the best value instead of
memcpy().  Use the same mechanism to loop over the offset array
everywhere to avoid an invalid best value.
tested by weerd@; OK claudio@

diff --git a/usr.sbin/ntpd/client.c b/usr.sbin/ntpd/client.c
index db10c8a..4ce70f5 100644 (file)
--- a/usr.sbin/ntpd/client.c
+++ b/usr.sbin/ntpd/client.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: client.c,v 1.115 2021/03/18 11:06:41 bluhm Exp $ */
+/*     $OpenBSD: client.c,v 1.116 2021/04/21 09:38:11 bluhm Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -473,7 +473,7 @@ client_dispatch(struct ntp_peer *p, u_int8_t settime, u_int8_t automatic)
 int
 client_update(struct ntp_peer *p)
 {
-       int     i, best = 0, good = 0;
+       int     shift, best = -1, good = 0;
 
        /*
         * clock filter
@@ -482,27 +482,22 @@ client_update(struct ntp_peer *p)
         * invalidate it and all older ones
         */
 
-       for (i = 0; good == 0 && i < OFFSET_ARRAY_SIZE; i++)
-               if (p->reply[i].good) {
+       for (shift = 0; shift < OFFSET_ARRAY_SIZE; shift++)
+               if (p->reply[shift].good) {
                        good++;
-                       best = i;
+                       if (best == -1 ||
+                           p->reply[shift].delay < p->reply[best].delay)
+                               best = shift;
                }
 
-       for (; i < OFFSET_ARRAY_SIZE; i++)
-               if (p->reply[i].good) {
-                       good++;
-                       if (p->reply[i].delay < p->reply[best].delay)
-                               best = i;
-               }
-
-       if (good < 8)
+       if (best == -1 || good < 8)
                return (-1);
 
-       memcpy(&p->update, &p->reply[best], sizeof(p->update));
+       p->update = p->reply[best];
        if (priv_adjtime() == 0) {
-               for (i = 0; i < OFFSET_ARRAY_SIZE; i++)
-                       if (p->reply[i].rcvd <= p->reply[best].rcvd)
-                               p->reply[i].good = 0;
+               for (shift = 0; shift < OFFSET_ARRAY_SIZE; shift++)
+                       if (p->reply[shift].rcvd <= p->reply[best].rcvd)
+                               p->reply[shift].good = 0;
        }
        return (0);
 }

diff --git a/usr.sbin/ntpd/control.c b/usr.sbin/ntpd/control.c
index 751e9cb..b86e615 100644 (file)
--- a/usr.sbin/ntpd/control.c
+++ b/usr.sbin/ntpd/control.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: control.c,v 1.18 2020/02/12 19:14:56 otto Exp $ */
+/*     $OpenBSD: control.c,v 1.19 2021/04/21 09:38:11 bluhm Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -341,7 +341,7 @@ build_show_peer(struct ctl_show_peer *cp, struct ntp_peer *p)
        const char      *a = "not resolved";
        const char      *pool = "", *addr_head_name = "";
        const char      *auth = "";
-       u_int8_t         shift, best, validdelaycnt, jittercnt;
+       int              shift, best = -1, validdelaycnt = 0, jittercnt = 0;
        time_t           now;
 
        now = getmonotime();
@@ -360,14 +360,14 @@ build_show_peer(struct ctl_show_peer *cp, struct ntp_peer *p)
        snprintf(cp->peer_desc, sizeof(cp->peer_desc),
            "%s %s%s%s", a, pool, addr_head_name, auth);
 
-       validdelaycnt = best = 0;
        cp->offset = cp->delay = 0.0;
        for (shift = 0; shift < OFFSET_ARRAY_SIZE; shift++) {
                if (p->reply[shift].delay > 0.0) {
                        cp->offset += p->reply[shift].offset;
                        cp->delay += p->reply[shift].delay;
 
-                       if (p->reply[shift].delay < p->reply[best].delay)
+                       if (best == -1 ||
+                           p->reply[shift].delay < p->reply[best].delay)
                                best = shift;
 
                        validdelaycnt++;
@@ -379,18 +379,19 @@ build_show_peer(struct ctl_show_peer *cp, struct ntp_peer *p)
                cp->delay /= validdelaycnt;
        }
 
-       jittercnt = 0;
        cp->jitter = 0.0;
-       for (shift = 0; shift < OFFSET_ARRAY_SIZE; shift++) {
-               if (p->reply[shift].delay > 0.0 && shift != best) {
-                       cp->jitter += square(p->reply[shift].delay -
-                           p->reply[best].delay);
-                       jittercnt++;
+       if (best != -1) {
+               for (shift = 0; shift < OFFSET_ARRAY_SIZE; shift++) {
+                       if (p->reply[shift].delay > 0.0 && shift != best) {
+                               cp->jitter += square(p->reply[shift].delay -
+                                   p->reply[best].delay);
+                               jittercnt++;
+                       }
                }
+               if (jittercnt > 1)
+                       cp->jitter /= jittercnt;
+               cp->jitter = sqrt(cp->jitter);
        }
-       if (jittercnt > 1)
-               cp->jitter /= jittercnt;
-       cp->jitter = sqrt(cp->jitter);
 
        if (p->shift == 0)
                shift = OFFSET_ARRAY_SIZE - 1;