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

diff --git a/src/client/main.c b/src/client/main.c
index ede9290..3c017a4 100644 (file)
--- 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 (file)
--- 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(&regexp_option, "^\\s*([^ \t]+)\\s+(.+)$", REG_EXTENDED);  //
        CompileRegex(&regexp_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(&regexp_option);
        regfree(&regexp_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 (file)
--- a/src/common/config.h
+++ b/src/common/config.h
@@ -9,6 +9,8 @@
 #ifndef _CONFIG_H_
 #define _CONFIG_H_
 
+#include <stdbool.h>   // 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 (file)
--- a/src/server/common.h
+++ b/src/server/common.h
@@ -9,6 +9,8 @@
 #ifndef _COMMON_H_
 #define _COMMON_H_
 
+#include <stdbool.h>   // Because C
+
 #include <regex.h>
 #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 (file)
--- 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 (file)
--- 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);