From 7d387dd4d680a07065ddf8b554087a0be8ae0cd6 Mon Sep 17 00:00:00 2001
From: Nathaniel Wesley Filardo <nwfilardo@gmail.com>
Date: Sat, 27 Jul 2019 13:20:26 +0100
Subject: [PATCH] Simplify and tidy SNTP (#2700)

* list_ref can become LUA_REFNIL, because that's what rawgeti returns
  for LUA_NOREF.  Defensively guard for this, rather than falling into
  the sntp_dolookups loop with nil on the stack.

* set_repeat_mode should not call itself, but should rather always do
  what it's going to do and then optionally do the rest if directed.

* sntp_sync should not try to special case the single string argument:
  we should be queueing that name for DNS resolution, too.  Towards that
  end, if we are given a single string, build a table and make that the
  list_ref and call off to sntp_dolookups, just like we otherwise do.

FIXES: #2699
---
 app/modules/sntp.c | 46 +++++++++++++++++++---------------------------
 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/app/modules/sntp.c b/app/modules/sntp.c
index 97ae32a8bf..887129b0c3 100644
--- a/app/modules/sntp.c
+++ b/app/modules/sntp.c
@@ -627,7 +627,7 @@ static void sntp_dolookups (lua_State *L) {
   // Step through each element of the table, converting it to an address
   // at the end, start the lookups. If we have already looked everything up,
   // then move straight to sending the packets.
-  if (state->list_ref == LUA_NOREF) {
+  if ((state->list_ref == LUA_NOREF) || (state->list_ref == LUA_REFNIL)) {
     sntp_dosend();
     return;
   }
@@ -702,8 +702,16 @@ static char *state_init(lua_State *L) {
 
 static char *set_repeat_mode(lua_State *L, bool enable)
 {
+  if (repeat) {
+    os_timer_disarm (&repeat->timer);
+    luaL_unref (L, LUA_REGISTRYINDEX, repeat->sync_cb_ref);
+    luaL_unref (L, LUA_REGISTRYINDEX, repeat->err_cb_ref);
+    luaL_unref (L, LUA_REGISTRYINDEX, repeat->list_ref);
+    free(repeat);
+    repeat = NULL;
+  }
+
   if (enable) {
-    set_repeat_mode(L, FALSE);
     repeat = (sntp_repeat_t *) malloc(sizeof(sntp_repeat_t));
     if (!repeat) {
       return "no memory";
@@ -720,16 +728,8 @@ static char *set_repeat_mode(lua_State *L, bool enable)
       //The function on_long_timeout returns errors to the developer
       //My guess: Error reporting is a good thing, resume the timer.
     os_timer_arm(&repeat->timer, 1000 * 1000, 1);
-  } else {
-    if (repeat) {
-      os_timer_disarm (&repeat->timer);
-      luaL_unref (L, LUA_REGISTRYINDEX, repeat->sync_cb_ref);
-      luaL_unref (L, LUA_REGISTRYINDEX, repeat->err_cb_ref);
-      luaL_unref (L, LUA_REGISTRYINDEX, repeat->list_ref);
-      free(repeat);
-      repeat = NULL;
-    }
   }
+
   return NULL;
 }
 
@@ -791,22 +791,17 @@ static int sntp_sync (lua_State *L)
     if (lua_istable(L, 1)) {
       // Save a reference to the table
       lua_pushvalue(L, 1);
-      luaL_unref (L, LUA_REGISTRYINDEX, state->list_ref);
-      state->list_ref = luaL_ref(L, LUA_REGISTRYINDEX);
-      sntp_dolookups(L);
-      goto good_ret;
     } else {
       size_t l;
       const char *hostname = luaL_checklstring(L, 1, &l);
       if (l>128 || hostname == NULL)
         sync_err("need <128 hostname");
-      err_t err = dns_gethostbyname(hostname, get_free_server(), sntp_dns_found, state);
-      if (err == ERR_INPROGRESS) {
-        goto good_ret;
-      } else if (err == ERR_ARG)
-        sync_err("bad hostname");
 
-      server_count++;
+      /* Construct a singleton table containing the one server */
+      lua_newtable(L);
+      lua_pushnumber(L, 1);
+      lua_pushstring(L, hostname);
+      lua_settable(L, -3);
     }
   } else if (server_count == 0) {
     lua_newtable(L);
@@ -827,15 +822,12 @@ static int sntp_sync (lua_State *L)
         lua_settable(L, -3);
       }
     }
-    luaL_unref (L, LUA_REGISTRYINDEX, state->list_ref);
-    state->list_ref = luaL_ref(L, LUA_REGISTRYINDEX);
-    sntp_dolookups(L);
-    goto good_ret;
   }
 
-  sntp_dosend ();
+  luaL_unref (L, LUA_REGISTRYINDEX, state->list_ref);
+  state->list_ref = luaL_ref(L, LUA_REGISTRYINDEX);
+  sntp_dolookups(L);
 
-good_ret:
   if (!lua_isnoneornil(L, 4)) {
     set_repeat_mode(L, 1);
   }