cmdparse: add status for server and local command

Add an enum to describe the error in the parsed directive: missing
argument, invalid option, or invalid value.

Update the error messages in conf.c and client.c.

client.c

@@ -759,7 +759,7 @@ process_cmd_local(CMD_Request *msg, char *line)
   if (!strcmp(line, "off")) {
     on_off = 0;
   } else if (CPS_ParseLocal(line, &stratum, &orphan, &distance, &activate,
-                            &wait_synced, &wait_unsynced)) {
+                            &wait_synced, &wait_unsynced) == CPS_Success) {
     on_off = 1;
   } else {
     LOG(LOGS_ERR, "Invalid syntax for local command");
@@ -907,8 +907,9 @@ static int
 process_cmd_add_source(CMD_Request *msg, char *line)
 {
   CPS_NTP_Source data;
+  CPS_Status status;
   IPAddr ip_addr;
-  int result = 0, status, type;
+  int result = 0, type;
   const char *opt_name, *word;
   
   msg->command = htons(REQ_ADD_SOURCE);
@@ -929,10 +930,7 @@ process_cmd_add_source(CMD_Request *msg, char *line)
 
   status = CPS_ParseNTPSourceAdd(line, &data);
   switch (status) {
-    case 0:
+    case CPS_Success:
-      LOG(LOGS_ERR, "Invalid syntax for add command");
-      break;
-    default:
       /* Verify that the address is resolvable (chronyc and chronyd are
          assumed to be running on the same host) */
       if (strlen(data.name) >= sizeof (msg->data.ntp_source.name) ||
@@ -993,6 +991,15 @@ process_cmd_add_source(CMD_Request *msg, char *line)
 
       result = 1;
 
+      break;
+    case CPS_InvalidOption:
+      LOG(LOGS_ERR, "Invalid %s add command", "option in");
+      break;
+    case CPS_InvalidValue:
+      LOG(LOGS_ERR, "Invalid %s add command", "value in");
+      break;
+    default:
+      LOG(LOGS_ERR, "Invalid %s add command", "syntax for");
       break;
   }
 

cmdparse.c

@@ -39,7 +39,7 @@
 
 /* ================================================== */
 
-int
+CPS_Status
 CPS_ParseNTPSourceAdd(char *line, CPS_NTP_Source *src)
 {
   char *hostname, *cmd;
@@ -82,7 +82,7 @@ CPS_ParseNTPSourceAdd(char *line, CPS_NTP_Source *src)
   line = CPS_SplitWord(line);
 
   if (!*hostname)
-    return 0;
+    return CPS_MissingArgument;
 
   src->name = hostname;
 
@@ -104,17 +104,17 @@ CPS_ParseNTPSourceAdd(char *line, CPS_NTP_Source *src)
       src->params.connectivity = SRC_OFFLINE;
     } else if (!strcasecmp(cmd, "certset")) {
       if (sscanf(line, "%"SCNu32"%n", &src->params.cert_set, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "key")) {
       if (sscanf(line, "%"SCNu32"%n", &src->params.authkey, &n) != 1 ||
           src->params.authkey == INACTIVE_AUTHKEY)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "asymmetry")) {
       if (sscanf(line, "%lf%n", &src->params.asymmetry, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "extfield")) {
       if (sscanf(line, "%"SCNx32"%n", &ef_type, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
       switch (ef_type) {
         case NTP_EF_EXP_MONO_ROOT:
           src->params.ext_fields |= NTP_EF_FLAG_EXP_MONO_ROOT;
@@ -123,78 +123,78 @@ CPS_ParseNTPSourceAdd(char *line, CPS_NTP_Source *src)
           src->params.ext_fields |= NTP_EF_FLAG_EXP_NET_CORRECTION;
           break;
         default:
-          return 0;
+          return CPS_InvalidValue;
       }
     } else if (!strcasecmp(cmd, "filter")) {
       if (sscanf(line, "%d%n", &src->params.filter_length, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "ipv4")) {
       src->family = IPADDR_INET4;
     } else if (!strcasecmp(cmd, "ipv6")) {
       src->family = IPADDR_INET6;
     } else if (!strcasecmp(cmd, "maxdelay")) {
       if (sscanf(line, "%lf%n", &src->params.max_delay, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "maxdelayratio")) {
       if (sscanf(line, "%lf%n", &src->params.max_delay_ratio, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "maxdelaydevratio")) {
       if (sscanf(line, "%lf%n", &src->params.max_delay_dev_ratio, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "maxdelayquant")) {
       if (sscanf(line, "%lf%n", &src->params.max_delay_quant, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "maxpoll")) {
       if (sscanf(line, "%d%n", &src->params.maxpoll, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "maxsamples")) {
       if (sscanf(line, "%d%n", &src->params.max_samples, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "maxsources")) {
       if (sscanf(line, "%d%n", &src->params.max_sources, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "mindelay")) {
       if (sscanf(line, "%lf%n", &src->params.min_delay, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "minpoll")) {
       if (sscanf(line, "%d%n", &src->params.minpoll, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "minsamples")) {
       if (sscanf(line, "%d%n", &src->params.min_samples, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "minstratum")) {
       if (sscanf(line, "%d%n", &src->params.min_stratum, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "nts")) {
       src->params.nts = 1;
     } else if (!strcasecmp(cmd, "ntsport")) {
       if (sscanf(line, "%d%n", &src->params.nts_port, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "offset")) {
       if (sscanf(line, "%lf%n", &src->params.offset, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "port")) {
       if (sscanf(line, "%d%n", &src->port, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "polltarget")) {
       if (sscanf(line, "%d%n", &src->params.poll_target, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "presend")) {
       if (sscanf(line, "%d%n", &src->params.presend_minpoll, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "version")) {
       if (sscanf(line, "%d%n", &src->params.version, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "xleave")) {
       src->params.interleaved = 1;
     } else if ((sel_option = CPS_GetSelectOption(cmd)) != 0) {
       src->params.sel_options |= sel_option;
     } else {
-      return 0;
+      return CPS_InvalidOption;
     }
   }
 
-  return 1;
+  return CPS_Success;
 }
 
 /* ================================================== */
@@ -295,7 +295,7 @@ CPS_ParseAllowDeny(char *line, int *all, IPAddr *ip, int *subnet_bits)
 
 /* ================================================== */
 
-int
+CPS_Status
 CPS_ParseLocal(char *line, int *stratum, int *orphan, double *distance, double *activate,
                double *wait_synced, double *wait_unsynced)
 {
@@ -316,24 +316,24 @@ CPS_ParseLocal(char *line, int *stratum, int *orphan, double *distance, double *
     if (!strcasecmp(cmd, "stratum")) {
       if (sscanf(line, "%d%n", stratum, &n) != 1 ||
           *stratum >= NTP_MAX_STRATUM || *stratum <= 0)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "orphan")) {
       *orphan = 1;
       n = 0;
     } else if (!strcasecmp(cmd, "distance")) {
       if (sscanf(line, "%lf%n", distance, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "activate")) {
       if (sscanf(line, "%lf%n", activate, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "waitsynced")) {
       if (sscanf(line, "%lf%n", wait_synced, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else if (!strcasecmp(cmd, "waitunsynced")) {
       if (sscanf(line, "%lf%n", wait_unsynced, &n) != 1)
-        return 0;
+        return CPS_InvalidValue;
     } else {
-      return 0;
+      return CPS_InvalidOption;
     }
 
     line += n;
@@ -342,7 +342,7 @@ CPS_ParseLocal(char *line, int *stratum, int *orphan, double *distance, double *
   if (*wait_unsynced < 0.0)
     *wait_unsynced = *orphan ? 300 : 0.0;
 
-  return 1;
+  return CPS_Success;
 }
 
 /* ================================================== */

cmdparse.h

@@ -30,6 +30,13 @@
 #include "srcparams.h"
 #include "addressing.h"
 
+typedef enum {
+  CPS_Success,
+  CPS_InvalidValue,
+  CPS_InvalidOption,
+  CPS_MissingArgument,
+} CPS_Status;
+
 typedef struct {
   char *name;
   int family;
@@ -38,7 +45,7 @@ typedef struct {
 } CPS_NTP_Source;
 
 /* Parse a command to add an NTP server or peer */
-extern int CPS_ParseNTPSourceAdd(char *line, CPS_NTP_Source *src);
+extern CPS_Status CPS_ParseNTPSourceAdd(char *line, CPS_NTP_Source *src);
   
 /* Get an NTP/refclock select option */
 extern int CPS_GetSelectOption(char *option);
@@ -47,7 +54,7 @@ extern int CPS_GetSelectOption(char *option);
 extern int CPS_ParseAllowDeny(char *line, int *all, IPAddr *ip, int *subnet_bits);
 
 /* Parse a command to enable local reference */
-extern int CPS_ParseLocal(char *line, int *stratum, int *orphan, double *distance,
+extern CPS_Status CPS_ParseLocal(char *line, int *stratum, int *orphan, double *distance,
                                  double *activate, double *wait_synced, double *wait_unsynced);
 
 /* Remove extra white-space and comments */

conf.c

@@ -860,6 +860,7 @@ parse_ints(char *line, ARR_Instance array)
 static void
 parse_source(char *line, char *type, int fatal)
 {
+  CPS_Status status;
   NTP_Source source;
 
   if (strcasecmp(type, "peer") == 0) {
@@ -880,9 +881,13 @@ parse_source(char *line, char *type, int fatal)
   /* Avoid comparing uninitialized data in compare_sources() */
   memset(&source.params, 0, sizeof (source.params));
 
-  if (!CPS_ParseNTPSourceAdd(line, &source.params)) {
+  status = CPS_ParseNTPSourceAdd(line, &source.params);
-    if (fatal)
+  if (status != CPS_Success) {
-      command_parse_error();
+    if (fatal) {
+      other_parse_error("Invalid %s %s directive",
+                        status == CPS_InvalidOption ? "option in" :
+                        status == CPS_InvalidValue ? "value in" : "syntax for", type);
+    }
     return;
   }
 
@@ -1127,9 +1132,16 @@ parse_log(char *line)
 static void
 parse_local(char *line)
 {
-  if (!CPS_ParseLocal(line, &local_stratum, &local_orphan, &local_distance,
+  CPS_Status status;
-                      &local_activate, &local_wait_synced, &local_wait_unsynced))
+
-    command_parse_error();
+  status = CPS_ParseLocal(line, &local_stratum, &local_orphan, &local_distance,
+                          &local_activate, &local_wait_synced, &local_wait_unsynced);
+  if (status != CPS_Success) {
+    other_parse_error("Invalid %s %s directive",
+                      status == CPS_InvalidOption ? "option in" : "value in",
+                      processed_command);
+  }
+
   enable_local = 1;
 }