From 9caffbc7ed251136bab144a957058e7d9f470b00 Mon Sep 17 00:00:00 2001 From: John Hodge Date: Wed, 15 Apr 2015 14:24:57 +0800 Subject: [PATCH] All - Rework config API to allow default/optional values - Now returns a boolean success, and takes a pointer in which to write the successfully parsed value (unchanged on failure) - Also cleaned up argument processing to be flatter and slightly less hacky --- src/client/main.c | 119 +++++++++++++++++++++++--------------------- src/common/config.c | 65 ++++++++++++++++-------- src/common/config.h | 14 ++++-- src/server/common.h | 4 +- src/server/main.c | 58 +++++++++++++-------- src/server/server.c | 2 +- 6 files changed, 156 insertions(+), 106 deletions(-) diff --git a/src/client/main.c b/src/client/main.c index ede9290..3c017a4 100644 --- a/src/client/main.c +++ b/src/client/main.c @@ -618,14 +618,17 @@ int main(int argc, char *argv[]) return ret; // Load config file - Config_ParseFile(gsConfigFile); + // - Don't check return value, leads to defaults being used + if( Config_ParseFile(gsConfigFile) ) { + fprintf(stderr, "NOTICE: Loading of config file '%s' failed, using defaults\n", gsConfigFile); + } // Parse config values - if (!giDispenseServerSet) { - gsDispenseServer = Config_GetValue("dispense_server",0); + if( !giDispenseServerSet ) { + Config_GetValue_Str("dispense_server", &gsDispenseServer); } if (!giDispensePortSet) { - giDispensePort = Config_GetValue_Int("dispense_port",0); + Config_GetValue_Int("dispense_port", &giDispensePort); } @@ -844,11 +847,25 @@ int main(int argc, char *argv[]) int ParseArguments(int argc, char *argv[]) { + bool rest_free = false; for( int i = 1; i < argc; i ++ ) { char *arg = argv[i]; - if( arg[0] == '-' ) + // If it doesn't start with a '-', or -- has been seen + // XXX: Hack - If parsing "user type", don't parse - options + bool hack_usertype = (i > 2 && strcmp(argv[i-2], "user") == 0 && strcmp(argv[i-1], "type") == 0); + if( rest_free || arg[0] != '-' || hack_usertype ) + { + if( giTextArgc == MAX_TXT_ARGS ) + { + fprintf(stderr, "ERROR: Too many arguments\n"); + return RV_ARGUMENTS; + } + + gsTextArgs[giTextArgc++] = argv[i]; + } + else if( arg[1] != '-' ) { switch(arg[1]) { @@ -858,8 +875,6 @@ int ParseArguments(int argc, char *argv[]) exit(0); case 'c': - if( i > 2 && strcmp(argv[i-1], "type") == 0 ) - goto _default; if( i + 1 >= argc ) { fprintf(stderr, "%s: -c takes an argument\n", argv[0]); ShowUsage(); @@ -947,54 +962,47 @@ int ParseArguments(int argc, char *argv[]) case 'n': // Dry Run / read-only gbDryRun = 1; break; - case '-': - if( strcmp(argv[i], "--help") == 0 ) { - ShowUsage(); - exit(0); - } - else if( strcmp(argv[i], "--dry-run") == 0 ) { - gbDryRun = 1; - } - else if( strcmp(argv[i], "--drinks-only") == 0 ) { - giUIMode = UI_MODE_DRINKSONLY; - } - else if( strcmp(argv[i], "--can-select-all") == 0 ) { - gbDisallowSelectWithoutBalance = 0; - } - else { - fprintf(stderr, "%s: Unknown switch '%s'\n", argv[0], argv[i]); - ShowUsage(); - return RV_ARGUMENTS; - } - break; - default: _default: - // The first argument is not allowed to begin with 'i' - // (catches most bad flags) - if( giTextArgc == 0 ) { - fprintf(stderr, "%s: Unknown switch '%s'\n", argv[0], argv[i]); - ShowUsage(); - return RV_ARGUMENTS; - } - if( giTextArgc == MAX_TXT_ARGS ) - { - fprintf(stderr, "ERROR: Too many arguments\n"); - return RV_ARGUMENTS; - } - gsTextArgs[giTextArgc++] = argv[i]; - break; + default: + fprintf(stderr, "%s: Unknown switch '%s'\n", argv[0], argv[i]); + ShowUsage(); + return RV_ARGUMENTS; } continue; } - - if( giTextArgc == MAX_TXT_ARGS ) + else { - fprintf(stderr, "ERROR: Too many arguments\n"); - return RV_ARGUMENTS; + // '--' : Terminate argument processing (remainder is free) + if( arg[2] == '\0' ) { + rest_free = true; + } + else if( strcmp(arg, "--help") == 0 ) { + ShowUsage(); + exit(0); + } + else if( strcmp(arg, "--dry-run") == 0 ) { + gbDryRun = 1; + } + else if( strcmp(arg, "--drinks-only") == 0 ) { + giUIMode = UI_MODE_DRINKSONLY; + } + else if( strcmp(arg, "--can-select-all") == 0 ) { + gbDisallowSelectWithoutBalance = 0; + } + else if( strcmp(arg, "--configfile") == 0 ) { + if( i + 1 >= argc ) { + fprintf(stderr, "%s: %s takes an argument\n", argv[0], arg); + ShowUsage(); + return RV_ARGUMENTS; + } + gsConfigFile = argv[++i]; + } + else { + fprintf(stderr, "%s: Unknown switch '%s'\n", argv[0], arg); + ShowUsage(); + return RV_ARGUMENTS; + } } - - gsTextArgs[giTextArgc++] = argv[i]; - } return 0; } @@ -1004,18 +1012,13 @@ int ParseArguments(int argc, char *argv[]) // --------------- char *trim(char *string) { - int i; - + // Increment pointer while it points to a space while( isspace(*string) ) string ++; - for( i = strlen(string); i--; ) - { - if( isspace(string[i]) ) - string[i] = '\0'; - else - break; - } + // And repalce trailing spaces with NUL bytes + for( int i = strlen(string); i-- && isspace(string[i]); ) + string[i] = '\0'; return string; } diff --git a/src/common/config.c b/src/common/config.c index 563240a..ca6db69 100644 --- a/src/common/config.c +++ b/src/common/config.c @@ -38,7 +38,7 @@ struct sConfigKey }; // === PROTOTYPES === -void Config_ParseFile(const char *Filename); +//int Config_ParseFile(const char *Filename); void Config_AddValue(const char *Key, const char *Value); void Config_int_AddValueToKey(tConfigKey *Key, const char *Value); tConfigKey *Config_int_GetKey(const char *KeyName, int bCreate); @@ -49,9 +49,8 @@ const char *Config_GetValue(const char *KeyName, int Index); tConfigKey *gConfig; // === CODE === -void Config_ParseFile(const char *Filename) +int Config_ParseFile(const char *Filename) { - FILE *fp; char line[MAX_LINE_LEN]; regex_t regexp_option; regex_t regexp_empty; @@ -59,11 +58,11 @@ void Config_ParseFile(const char *Filename) CompileRegex(®exp_option, "^\\s*([^ \t]+)\\s+(.+)$", REG_EXTENDED); // CompileRegex(®exp_empty, "^\\s*$", REG_EXTENDED); // - fp = fopen(Filename, "r"); + FILE *fp = fopen(Filename, "r"); if(!fp) { fprintf(stderr, "Unable to open config file '%s'\n", Filename); perror("Config_ParseFile"); - exit(-1); + return 1; } while( fgets(line, sizeof(line), fp) ) @@ -104,6 +103,8 @@ void Config_ParseFile(const char *Filename) fclose(fp); regfree(®exp_option); regfree(®exp_empty); + + return 0; } void Config_AddValue(const char *Key, const char *Value) @@ -191,12 +192,11 @@ int Config_GetValueCount(const char *KeyName) return key->ValueCount; } -const char *Config_GetValue(const char *KeyName, int Index) +const tConfigValue *Config_int_GetValue(const char *KeyName, int Index) { - tConfigKey *key; tConfigValue *val; - key = Config_int_GetKey(KeyName, 0); + tConfigKey *key = Config_int_GetKey(KeyName, 0); if(!key) { fprintf(stderr, "Unknown key '%s'\n", KeyName); exit(1); @@ -208,15 +208,25 @@ const char *Config_GetValue(const char *KeyName, int Index) for( val = key->FirstValue; Index && val; val = val->Next, Index -- ); ASSERT(val != NULL); - + return val; +} + +const char *Config_GetValue_Idx(const char *KeyName, int Index) +{ + const tConfigValue* val = Config_int_GetValue(KeyName, Index); + if( !val ) return NULL; return val->Data; } -int Config_GetValue_Bool(const char *KeyName, int Index) +bool Config_GetValue_Str(const char *KeyName, const char** Value) +{ + const tConfigValue* val = Config_int_GetValue(KeyName, 0); + if(!val) return false; + *Value = val->Data; + return true; +} +bool str_to_bool(const char *val) { - const char *val = Config_GetValue(KeyName, Index); - if(!val) return -1; - if( atoi(val) == 1 ) return 1; if( val[0] == '0' && val[1] == '\0' ) return 0; @@ -225,19 +235,32 @@ int Config_GetValue_Bool(const char *KeyName, int Index) if( strcasecmp(val, "yes") == 0 ) return 1; if( strcasecmp(val, "no") == 0 ) return 0; + + // INVALID, TODO: Error message + return 0; +} +bool Config_GetValue_Bool(const char *KeyName, bool* Value) +{ + const tConfigValue* val = Config_int_GetValue(KeyName, 0); + if(!val) return false; - return -1; + *Value = str_to_bool(val->Data); + + return true; } -int Config_GetValue_Int(const char *KeyName, int Index) +bool Config_GetValue_Int(const char *KeyName, int* Value) { - int tmp; - const char *val = Config_GetValue(KeyName, Index); - if(!val) return -1; + const tConfigValue* val = Config_int_GetValue(KeyName, 0); + if(!val) return false; - if( (tmp = atoi(val)) ) return tmp; - if( val[0] == '0' && val[1] == '\0' ) return 0; + char* end; + int value = strtol(val->Data, &end, 0); + if( *end != '\0' ) { + return false; + } + *Value = value; - return -1; + return true; } diff --git a/src/common/config.h b/src/common/config.h index 901cad8..34d7dc6 100644 --- a/src/common/config.h +++ b/src/common/config.h @@ -9,6 +9,8 @@ #ifndef _CONFIG_H_ #define _CONFIG_H_ +#include // Because C + // === HELPER MACROS === #define _EXPSTR(x) #x #define EXPSTR(x) _EXPSTR(x) @@ -18,11 +20,15 @@ // --- Config Database --- -extern void Config_ParseFile(const char *Filename); +extern int Config_ParseFile(const char *Filename); + extern void Config_AddValue(const char *Key, const char *Value); + extern int Config_GetValueCount(const char *KeyName); -extern const char *Config_GetValue(const char *KeyName, int Index); -extern int Config_GetValue_Bool(const char *KeyName, int Index); -extern int Config_GetValue_Int(const char *KeyName, int Index); +extern const char *Config_GetValue_Idx(const char *KeyName, int Index); + +extern bool Config_GetValue_Str(const char *KeyName, const char** ValPtr); +extern bool Config_GetValue_Bool(const char *KeyName, bool* ValPtr); +extern bool Config_GetValue_Int(const char *KeyName, int* ValPtr); #endif diff --git a/src/server/common.h b/src/server/common.h index 1bdca84..34ae298 100644 --- a/src/server/common.h +++ b/src/server/common.h @@ -9,6 +9,8 @@ #ifndef _COMMON_H_ #define _COMMON_H_ +#include // Because C + #include #include "../cokebank.h" @@ -69,7 +71,7 @@ extern int giNumItems; extern tHandler *gaHandlers[]; extern int giNumHandlers; extern int giDebugLevel; -extern int gbNoCostMode; +extern bool gbNoCostMode; // === FUNCTIONS === extern void Items_UpdateFile(void); diff --git a/src/server/main.c b/src/server/main.c index 9359c4e..3e06230 100644 --- a/src/server/main.c +++ b/src/server/main.c @@ -27,7 +27,7 @@ extern void Init_Handlers(void); extern void Load_Itemlist(void); extern void Server_Start(void); -extern int gbServer_RunInBackground; +extern bool gbServer_RunInBackground; extern int giServer_Port; extern const char *gsItemListFile; extern const char *gsCoke_ModbusAddress; @@ -40,7 +40,7 @@ void *Periodic_Thread(void *Unused); // === GLOBALS === int giDebugLevel = 0; - int gbNoCostMode = 0; +bool gbNoCostMode = 0; const char *gsCokebankPath = "cokebank.db"; // - Functions called every 20s (or so) #define ciMaxPeriodics 10 @@ -74,8 +74,16 @@ int main(int argc, char *argv[]) for( i = 1; i < argc; i++ ) { char *arg = argv[i]; - if( arg[0] == '-' && arg[1] != '-') + if( arg[0] != '-' ) { + // No free arguments please + PrintUsage(argv[0]); + return -1; + } + else if( arg[1] != '-' ) + { + // Single character arguments + // - TODO: Process in a loop switch(arg[1]) { case 'f': @@ -94,8 +102,9 @@ int main(int argc, char *argv[]) return -1; } } - else if( arg[0] == '-' && arg[1] == '-' ) + else { + // Long arguments if( strcmp(arg, "--configfile") == 0 ) { if( i + 1 >= argc ) return -1; config_file = argv[++i]; @@ -113,28 +122,35 @@ int main(int argc, char *argv[]) return -1; } } - else - { - // Usage Error - PrintUsage(argv[0]); - return -1; - } } - Config_ParseFile( config_file ); + if( Config_ParseFile( config_file ) ) { + fprintf(stderr, "NOTICE: Loading of config file '%s' failed, using defaults\n", config_file); + } // Parse config values - gbServer_RunInBackground = Config_GetValue_Bool("daemonise", 0); - gsCokebankPath = Config_GetValue("cokebank_database", 0); - gsDoor_SerialPort = Config_GetValue("door_serial_port", 0); - giServer_Port = Config_GetValue_Int("server_port", 0); - gsItemListFile = Config_GetValue("items_file", 0); - - gbNoCostMode = (Config_GetValue_Bool("test_mode", 0) == 1); - gbSyslogEnabled = (Config_GetValue_Bool("disable_syslog", 0) == 0); + { + bool rv = true; + #define REQ_CFG(variable, type, name) rv = Config_GetValue_##type(name, &variable) && rv + #define OPT_CFG(variable, type, name) Config_GetValue_##type(name, &variable) + OPT_CFG(gbServer_RunInBackground, Bool, "daemonise"); + OPT_CFG(giServer_Port, Int, "server_port"); + + REQ_CFG(gsCokebankPath, Str, "cokebank_database"); + REQ_CFG(gsItemListFile, Str, "items_file"); - gsCoke_ModbusAddress = Config_GetValue("coke_modbus_address", 0); - giCoke_ModbusPort = Config_GetValue_Int("coke_modbus_port", 0); + OPT_CFG(gsDoor_SerialPort, Str, "door_serial_port"); + REQ_CFG(gsCoke_ModbusAddress, Str, "coke_modbus_address"); + OPT_CFG(giCoke_ModbusPort, Int, "coke_modbus_port"); + + OPT_CFG(gbNoCostMode, Bool, "test_mode"); + OPT_CFG(gbSyslogEnabled, Bool, "disable_syslog"); + + if( !rv ) { + fprintf(stderr, "ERROR: Some required configuration items were missing\n"); + return -1; + } + } // - Cleanly tear down the server on SIGINT/SIGTERM signal(SIGINT, sigint_handler); diff --git a/src/server/server.c b/src/server/server.c index 11846bb..b09c794 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -148,7 +148,7 @@ void Server_Start(void) gaServer_TrustedHosts = malloc(giServer_NumTrustedHosts * sizeof(*gaServer_TrustedHosts)); for( int i = 0; i < giServer_NumTrustedHosts; i ++ ) { - const char *addr = Config_GetValue("trusted_host", i); + const char *addr = Config_GetValue_Idx("trusted_host", i); if( inet_aton(addr, &gaServer_TrustedHosts[i]) == 0 ) { fprintf(stderr, "Invalid IP address '%s'\n", addr); -- 2.20.1