From e694ae769a4942e35450cc19c6ab065c680edada Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Wed, 19 Mar 2025 12:07:29 +0100 Subject: [PATCH] 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 | 19 ++++++++++----- cmdparse.c | 70 +++++++++++++++++++++++++++--------------------------- cmdparse.h | 13 +++++++--- conf.c | 24 ++++++++++++++----- 4 files changed, 76 insertions(+), 50 deletions(-) diff --git a/client.c b/client.c index 32ca958c..fd227e84 100644 --- a/client.c +++ b/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: - LOG(LOGS_ERR, "Invalid syntax for add command"); - break; - default: + case CPS_Success: /* 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; } diff --git a/cmdparse.c b/cmdparse.c index 1cf3740b..19078ce1 100644 --- a/cmdparse.c +++ b/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; } /* ================================================== */ diff --git a/cmdparse.h b/cmdparse.h index dd82a394..960c4585 100644 --- a/cmdparse.h +++ b/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,8 +54,8 @@ 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, - double *activate, double *wait_synced, double *wait_unsynced); +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 */ extern void CPS_NormalizeLine(char *line); diff --git a/conf.c b/conf.c index 8b468f1a..0b7a678c 100644 --- a/conf.c +++ b/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)) { - if (fatal) - command_parse_error(); + status = CPS_ParseNTPSourceAdd(line, &source.params); + if (status != CPS_Success) { + 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, - &local_activate, &local_wait_synced, &local_wait_unsynced)) - command_parse_error(); + CPS_Status status; + + 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; } -- 2.47.2