Refactor Ini interface to expose String values instead of Variant.

Variants were being used to expose String and StringList types but this can be done more simply with an additional method.

Using only strings also allows for a more efficient implementation down the road.

doc/xml/release.xml

@@ -33,6 +33,10 @@
                         <p>Add <code>unsigned int</code> <code>Variant</code> type and update code to use it.</p>
                     </release-item>
 
+                    <release-item>
+                        <p>Refactor <code>Ini</code> interface to expose <code>String</code> values instead of <code>Variant</code>.</p>
+                    </release-item>
+
                     <release-item>
                         <p>Refactor <code>main()</code> as a <code>switch()</code> statement.</p>
                     </release-item>

src/Makefile

@@ -284,7 +284,7 @@ common/exit.o: common/exit.c command/command.h common/assert.h common/debug.h co
 common/fork.o: common/fork.c common/assert.h common/debug.h common/error.auto.h common/error.h common/log.h common/logLevel.h common/stackTrace.h common/type/convert.h
 	$(CC) $(CFLAGS) -c common/fork.c -o common/fork.o
 
-common/ini.o: common/ini.c common/assert.h common/debug.h common/error.auto.h common/error.h common/ini.h common/logLevel.h common/memContext.h common/stackTrace.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/variant.h common/type/variantList.h
+common/ini.o: common/ini.c common/assert.h common/debug.h common/error.auto.h common/error.h common/ini.h common/io/filter/filter.h common/io/filter/group.h common/io/write.h common/logLevel.h common/memContext.h common/stackTrace.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/variant.h common/type/variantList.h
 	$(CC) $(CFLAGS) -c common/ini.c -o common/ini.o
 
 common/io/bufferRead.o: common/io/bufferRead.c common/assert.h common/debug.h common/error.auto.h common/error.h common/io/bufferRead.h common/io/filter/filter.h common/io/filter/group.h common/io/read.h common/io/read.intern.h common/log.h common/logLevel.h common/memContext.h common/stackTrace.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/variant.h common/type/variantList.h

src/common/ini.c

@@ -46,12 +46,13 @@ iniNew(void)
 Internal function to get an ini value
 ***********************************************************************************************************************************/
 static const Variant *
-iniGetInternal(const Ini *this, const String *section, const String *key)
+iniGetInternal(const Ini *this, const String *section, const String *key, bool required)
 {
     FUNCTION_TEST_BEGIN();
         FUNCTION_TEST_PARAM(INI, this);
         FUNCTION_TEST_PARAM(STRING, section);
         FUNCTION_TEST_PARAM(STRING, key);
+        FUNCTION_TEST_PARAM(BOOL, required);
     FUNCTION_TEST_END();
 
     ASSERT(this != NULL);
@@ -67,13 +68,17 @@ iniGetInternal(const Ini *this, const String *section, const String *key)
     if (sectionKv != NULL)
         result = kvGet(sectionKv, VARSTR(key));
 
+    // If value is null and required then error
+    if (result == NULL && required)
+        THROW_FMT(FormatError, "section '%s', key '%s' does not exist", strPtr(section), strPtr(key));
+
     FUNCTION_TEST_RETURN(result);
 }
 
 /***********************************************************************************************************************************
 Get an ini value -- error if it does not exist
 ***********************************************************************************************************************************/
-const Variant *
+const String *
 iniGet(const Ini *this, const String *section, const String *key)
 {
     FUNCTION_TEST_BEGIN();
@@ -87,26 +92,22 @@ iniGet(const Ini *this, const String *section, const String *key)
     ASSERT(key != NULL);
 
     // Get the value
-    const Variant *result = iniGetInternal(this, section, key);
+    const Variant *result = iniGetInternal(this, section, key, true);
 
-    // If value is null replace it with default
+    FUNCTION_TEST_RETURN(varStr(result));
-    if (result == NULL)
-        THROW_FMT(FormatError, "section '%s', key '%s' does not exist", strPtr(section), strPtr(key));
-
-    FUNCTION_TEST_RETURN(result);
 }
 
 /***********************************************************************************************************************************
 Get an ini value -- if it does not exist then return specified default
 ***********************************************************************************************************************************/
-const Variant *
+const String *
-iniGetDefault(const Ini *this, const String *section, const String *key, const Variant *defaultValue)
+iniGetDefault(const Ini *this, const String *section, const String *key, const String *defaultValue)
 {
     FUNCTION_TEST_BEGIN();
         FUNCTION_TEST_PARAM(INI, this);
         FUNCTION_TEST_PARAM(STRING, section);
         FUNCTION_TEST_PARAM(STRING, key);
-        FUNCTION_TEST_PARAM(VARIANT, defaultValue);
+        FUNCTION_TEST_PARAM(STRING, defaultValue);
     FUNCTION_TEST_END();
 
     ASSERT(this != NULL);
@@ -114,13 +115,53 @@ iniGetDefault(const Ini *this, const String *section, const String *key, const V
     ASSERT(key != NULL);
 
     // Get the value
-    const Variant *result = iniGetInternal(this, section, key);
+    const Variant *result = iniGetInternal(this, section, key, false);
 
-    // If value is null replace it with default
+    FUNCTION_TEST_RETURN(result == NULL ? defaultValue : varStr(result));
-    if (result == NULL)
+}
-        result = defaultValue;
 
-    FUNCTION_TEST_RETURN(result);
+/***********************************************************************************************************************************
+Internal function to get an ini value list
+***********************************************************************************************************************************/
+StringList *
+iniGetList(const Ini *this, const String *section, const String *key)
+{
+    FUNCTION_TEST_BEGIN();
+        FUNCTION_TEST_PARAM(INI, this);
+        FUNCTION_TEST_PARAM(STRING, section);
+        FUNCTION_TEST_PARAM(STRING, key);
+    FUNCTION_TEST_END();
+
+    ASSERT(this != NULL);
+    ASSERT(section != NULL);
+    ASSERT(key != NULL);
+
+    // Get the value
+    const Variant *result = iniGetInternal(this, section, key, false);
+
+    FUNCTION_TEST_RETURN(result == NULL ? false : strLstNewVarLst(varVarLst(result)));
+}
+
+/***********************************************************************************************************************************
+Internal function to get an ini value list
+***********************************************************************************************************************************/
+bool
+iniSectionKeyIsList(const Ini *this, const String *section, const String *key)
+{
+    FUNCTION_TEST_BEGIN();
+        FUNCTION_TEST_PARAM(INI, this);
+        FUNCTION_TEST_PARAM(STRING, section);
+        FUNCTION_TEST_PARAM(STRING, key);
+    FUNCTION_TEST_END();
+
+    ASSERT(this != NULL);
+    ASSERT(section != NULL);
+    ASSERT(key != NULL);
+
+    // Get the value
+    const Variant *result = iniGetInternal(this, section, key, true);
+
+    FUNCTION_TEST_RETURN(varType(result) == varTypeVariantList);
 }
 
 /***********************************************************************************************************************************
@@ -249,11 +290,8 @@ iniParse(Ini *this, const String *content)
                             if (strSize(key) == 0)
                                 THROW_FMT(FormatError, "key is zero-length at line %u: %s", lineIdx++, linePtr);
 
-                            // Extract the value
-                            const Variant *value = VARSTR(strTrim(strNew(lineEqual + 1)));
-
                             // Store the section/key/value
-                            iniSet(this, section, key, value);
+                            iniSet(this, section, key, strTrim(strNew(lineEqual + 1)));
                         }
                     }
                 }
@@ -270,13 +308,13 @@ iniParse(Ini *this, const String *content)
 Set an ini value
 ***********************************************************************************************************************************/
 void
-iniSet(Ini *this, const String *section, const String *key, const Variant *value)
+iniSet(Ini *this, const String *section, const String *key, const String *value)
 {
     FUNCTION_TEST_BEGIN();
         FUNCTION_TEST_PARAM(INI, this);
         FUNCTION_TEST_PARAM(STRING, section);
         FUNCTION_TEST_PARAM(STRING, key);
-        FUNCTION_TEST_PARAM(VARIANT, value);
+        FUNCTION_TEST_PARAM(STRING, value);
     FUNCTION_TEST_END();
 
     ASSERT(this != NULL);
@@ -292,7 +330,7 @@ iniSet(Ini *this, const String *section, const String *key, const Variant *value
         if (sectionKv == NULL)
             sectionKv = kvPutKv(this->store, sectionKey);
 
-        kvAdd(sectionKv, VARSTR(key), value);
+        kvAdd(sectionKv, VARSTR(key), VARSTR(value));
     }
     MEM_CONTEXT_TEMP_END();
 

src/common/ini.h

@@ -9,22 +9,42 @@ Ini object
 ***********************************************************************************************************************************/
 typedef struct Ini Ini;
 
+#include "common/io/write.h"
 #include "common/type/variant.h"
 
+/***********************************************************************************************************************************
+Constructor
+***********************************************************************************************************************************/
+Ini *iniNew(void);
+
 /***********************************************************************************************************************************
 Functions
 ***********************************************************************************************************************************/
-Ini *iniNew(void);
+void iniParse(Ini *this, const String *content);
-const Variant *iniGet(const Ini *this, const String *section, const String *key);
+void iniSet(Ini *this, const String *section, const String *key, const String *value);
-const Variant *iniGetDefault(const Ini *this, const String *section, const String *key, const Variant *defaultValue);
+
+/***********************************************************************************************************************************
+Getters
+***********************************************************************************************************************************/
+const String *iniGet(const Ini *this, const String *section, const String *key);
+const String *iniGetDefault(const Ini *this, const String *section, const String *key, const String *defaultValue);
+StringList *iniGetList(const Ini *this, const String *section, const String *key);
+bool iniSectionKeyIsList(const Ini *this, const String *section, const String *key);
 StringList *iniSectionKeyList(const Ini *this, const String *section);
 StringList *iniSectionList(const Ini *this);
-void iniParse(Ini *this, const String *content);
-void iniSet(Ini *this, const String *section, const String *key, const Variant *value);
-void iniFree(Ini *this);
 String *iniFileName(const Ini *this);
 bool iniFileExists(const Ini *this);
 
+/***********************************************************************************************************************************
+Constructor
+***********************************************************************************************************************************/
+void iniFree(Ini *this);
+
+/***********************************************************************************************************************************
+Destructor
+***********************************************************************************************************************************/
+void iniFree(Ini *this);
+
 /***********************************************************************************************************************************
 Macros for function logging
 ***********************************************************************************************************************************/

src/config/parse.c

@@ -759,39 +759,43 @@ configParse(unsigned int argListSize, const char *argList[], bool resetLogLevel)
                         if (parseOptionList[optionId].found)
                             continue;
 
-                        // Get the option value
-                        const Variant *value = iniGetDefault(config, section, key, NULL);
-
-                        if (varType(value) == varTypeString && strSize(varStr(value)) == 0)
-                        {
-                            THROW_FMT(
-                                OptionInvalidValueError, "section '%s', key '%s' must have a value", strPtr(section), strPtr(key));
-                        }
-
                         parseOptionList[optionId].found = true;
                         parseOptionList[optionId].source = cfgSourceConfig;
 
-                        if (varType(value) == varTypeVariantList)
+                        // Process list
+                        if (iniSectionKeyIsList(config, section, key))
                         {
                             // Error if the option cannot be specified multiple times
                             if (!cfgDefOptionMulti(optionDefId))
                                 THROW_FMT(OptionInvalidError, "option '%s' cannot be set multiple times", cfgOptionName(optionId));
 
-                            parseOptionList[optionId].valueList = strLstNewVarLst(varVarLst(value));
+                            parseOptionList[optionId].valueList = iniGetList(config, section, key);
                         }
-                        // Convert boolean
-                        else if (cfgDefOptionType(optionDefId) == cfgDefOptTypeBoolean)
-                        {
-                            if (strcasecmp(strPtr(varStr(value)), "n") == 0)
-                                parseOptionList[optionId].negate = true;
-                            else if (strcasecmp(strPtr(varStr(value)), "y") != 0)
-                                THROW_FMT(OptionInvalidValueError, "boolean option '%s' must be 'y' or 'n'", strPtr(key));
-                        }
-                        // Else add the string value
                         else
                         {
-                            parseOptionList[optionId].valueList = strLstNew();
+                            // Get the option value
-                            strLstAdd(parseOptionList[optionId].valueList, varStr(value));
+                            const String *value = iniGet(config, section, key);
+
+                            if (strSize(value) == 0)
+                            {
+                                THROW_FMT(
+                                    OptionInvalidValueError, "section '%s', key '%s' must have a value", strPtr(section),
+                                    strPtr(key));
+                            }
+
+                            if (cfgDefOptionType(optionDefId) == cfgDefOptTypeBoolean)
+                            {
+                                if (strEqZ(value, "n"))
+                                    parseOptionList[optionId].negate = true;
+                                else if (!strEqZ(value, "y"))
+                                    THROW_FMT(OptionInvalidValueError, "boolean option '%s' must be 'y' or 'n'", strPtr(key));
+                            }
+                            // Else add the string value
+                            else
+                            {
+                                parseOptionList[optionId].valueList = strLstNew();
+                                strLstAdd(parseOptionList[optionId].valueList, value);
+                            }
                         }
                     }
                 }

src/info/info.c

@@ -89,7 +89,7 @@ infoHash(const Ini *ini)
                     cryptoHashProcessC(result, (const unsigned char *)"\"", 1);
                     cryptoHashProcessStr(result, key);
                     cryptoHashProcessC(result, (const unsigned char *)"\":", 2);
-                    cryptoHashProcessStr(result, varStr(iniGet(ini, section, strLstGet(keyList, keyIdx))));
+                    cryptoHashProcessStr(result, iniGet(ini, section, strLstGet(keyList, keyIdx)));
                     if ((keyListSize > 1) && (keyIdx < keyListSize - 1))
                         cryptoHashProcessC(result, (const unsigned char *)",", 1);
                 }
@@ -159,7 +159,7 @@ infoLoad(Info *this, const Storage *storage, bool copyFile, CipherType cipherTyp
         iniParse(this->ini, strNewBuf(buffer));
 
         // Make sure the ini is valid by testing the checksum
-        const String *infoChecksum = varStr(iniGet(this->ini, INFO_SECTION_BACKREST_STR, INFO_KEY_CHECKSUM_STR));
+        const String *infoChecksum = iniGet(this->ini, INFO_SECTION_BACKREST_STR, INFO_KEY_CHECKSUM_STR);
 
         CryptoHash *hash = infoHash(this->ini);
 
@@ -178,11 +178,11 @@ infoLoad(Info *this, const Storage *storage, bool copyFile, CipherType cipherTyp
         }
 
         // Make sure that the format is current, otherwise error
-        if (varIntForce(iniGet(this->ini, INFO_SECTION_BACKREST_STR, INFO_KEY_FORMAT_STR)) != REPOSITORY_FORMAT)
+        if (varIntForce(VARSTR(iniGet(this->ini, INFO_SECTION_BACKREST_STR, INFO_KEY_FORMAT_STR))) != REPOSITORY_FORMAT)
         {
             THROW_FMT(
                 FormatError, "invalid format in '%s', expected %d but found %d", strPtr(fileName), REPOSITORY_FORMAT,
-                varIntForce(iniGet(this->ini, INFO_SECTION_BACKREST_STR, INFO_KEY_FORMAT_STR)));
+                varIntForce(VARSTR(iniGet(this->ini, INFO_SECTION_BACKREST_STR, INFO_KEY_FORMAT_STR))));
         }
     }
     MEM_CONTEXT_TEMP_END();
@@ -255,7 +255,7 @@ infoNew(const Storage *storage, const String *fileName, CipherType cipherType, c
         TRY_END();
 
         // Load the cipher passphrase if it exists
-        const String *cipherPass = varStr(iniGetDefault(this->ini, INFO_SECTION_CIPHER_STR, INFO_KEY_CIPHER_PASS_STR, NULL));
+        const String *cipherPass = iniGetDefault(this->ini, INFO_SECTION_CIPHER_STR, INFO_KEY_CIPHER_PASS_STR, NULL);
 
         if (cipherPass != NULL)
             this->cipherPass = strSubN(cipherPass, 1, strSize(cipherPass) - 2);

src/info/infoBackup.c

@@ -100,7 +100,7 @@ infoBackupNew(const Storage *storage, const String *fileName, bool ignoreMissing
             for (unsigned int backupLabelIdx = 0; backupLabelIdx < strLstSize(backupLabelList); backupLabelIdx++)
             {
                 const String *backupLabelKey = strLstGet(backupLabelList, backupLabelIdx);
-                const KeyValue *backupKv = varKv(jsonToVar(varStr(iniGet(infoIni, backupCurrentSection, backupLabelKey))));
+                const KeyValue *backupKv = varKv(jsonToVar(iniGet(infoIni, backupCurrentSection, backupLabelKey)));
 
                 InfoBackupData infoBackupData =
                 {

src/info/infoPg.c

@@ -83,7 +83,7 @@ infoPgNew(const Storage *storage, const String *fileName, InfoPgType type, Ciphe
             const StringList *pgHistoryKey = iniSectionKeyList(infoPgIni, INFO_SECTION_DB_HISTORY_STR);
 
             // Get the current history id
-            unsigned int pgId = (unsigned int)varUInt64Force(iniGet(infoPgIni, INFO_SECTION_DB_STR, varStr(INFO_KEY_DB_ID_VAR)));
+            unsigned int pgId = varUIntForce(VARSTR(iniGet(infoPgIni, INFO_SECTION_DB_STR, varStr(INFO_KEY_DB_ID_VAR))));
 
             // History must include at least one item or the file is corrupt
             ASSERT(strLstSize(pgHistoryKey) > 0);
@@ -95,7 +95,7 @@ infoPgNew(const Storage *storage, const String *fileName, InfoPgType type, Ciphe
             {
                 // Load JSON data into a KeyValue
                 const KeyValue *pgDataKv = varKv(
-                    jsonToVar(varStr(iniGet(infoPgIni, INFO_SECTION_DB_HISTORY_STR, strLstGet(pgHistoryKey, pgHistoryIdx)))));
+                    jsonToVar(iniGet(infoPgIni, INFO_SECTION_DB_HISTORY_STR, strLstGet(pgHistoryKey, pgHistoryIdx))));
 
                 // Get db values that are common to all info files
                 InfoPgData infoPgData =

test/src/module/common/iniTest.c

@@ -29,25 +29,30 @@ testRun(void)
 
         TEST_ASSIGN(ini, iniNew(), "new ini");
 
-        TEST_RESULT_VOID(iniSet(ini, strNew("section1"), strNew("key1"), varNewInt(11)), "set section, key, int");
+        TEST_RESULT_VOID(iniSet(ini, strNew("section1"), strNew("key1"), strNew("11")), "set section, key");
-        TEST_RESULT_VOID(iniSet(ini, strNew("section1"), strNew("key2"), varNewDbl(1.234)), "set section, key, dbl");
+        TEST_RESULT_VOID(iniSet(ini, strNew("section1"), strNew("key2"), strNew("1.234")), "set section, key");
-        TEST_RESULT_INT(varInt(iniGet(ini, strNew("section1"), strNew("key1"))), 11, "get section, key, int");
+        TEST_RESULT_STR(strPtr(iniGet(ini, strNew("section1"), strNew("key1"))), "11", "get section, key");
-        TEST_RESULT_DOUBLE(varDbl(iniGet(ini, strNew("section1"), strNew("key2"))), 1.234, "get section, key, dbl");
+        TEST_RESULT_STR(strPtr(iniGet(ini, strNew("section1"), strNew("key2"))), "1.234", "get section, key");
 
         TEST_ERROR(iniGet(ini, strNew("section2"), strNew("key2")), FormatError, "section 'section2', key 'key2' does not exist");
 
-        TEST_RESULT_INT(varInt(iniGetDefault(ini, strNew("section1"), strNew("key1"), NULL)), 11, "get section, key, int");
+        TEST_RESULT_STR(strPtr(iniGetDefault(ini, strNew("section1"), strNew("key1"), NULL)), "11", "get section, key, int");
         TEST_RESULT_PTR(iniGetDefault(ini, strNew("section2"), strNew("key2"), NULL), NULL, "get section, key, NULL");
-        TEST_RESULT_BOOL(
+        TEST_RESULT_STR(
-                varBool(iniGetDefault(ini, strNew("section3"), strNew("key3"), varNewBool(true))), true, "get section, key, bool");
+                strPtr(iniGetDefault(ini, strNew("section3"), strNew("key3"), strNew("true"))), "true", "get section, key, bool");
 
         TEST_RESULT_INT(strLstSize(iniSectionKeyList(ini, strNew("bogus"))), 0, "get keys for missing section");
         TEST_RESULT_STR(strPtr(strLstJoin(iniSectionKeyList(ini, strNew("section1")), "|")), "key1|key2", "get keys for section");
 
-        TEST_RESULT_VOID(iniSet(ini, strNew("section2"), strNew("key2"), varNewInt(2)), "set section2, key, int");
+        TEST_RESULT_VOID(iniSet(ini, strNew("section2"), strNew("key2"), strNew("2")), "set section2, key");
         TEST_RESULT_INT(strLstSize(iniSectionList(ini)), 2, "number of sections");
         TEST_RESULT_STR(strPtr(strLstJoin(iniSectionList(ini), "|")), "section1|section2", "get sections");
 
+        TEST_RESULT_BOOL(iniSectionKeyIsList(ini, strNew("section1"), strNew("key1")), false, "single value is not list");
+        TEST_RESULT_VOID(iniSet(ini, strNew("section2"), strNew("key2"), strNew("7")), "set section2, key");
+        TEST_RESULT_BOOL(iniSectionKeyIsList(ini, strNew("section2"), strNew("key2")), true, "section2, key2 is a list");
+        TEST_RESULT_STR(strPtr(strLstJoin(iniGetList(ini, strNew("section2"), strNew("key2")), "|")), "2|7", "get list");
+
         TEST_RESULT_VOID(iniFree(ini), "free ini");
     }
 
@@ -80,8 +85,8 @@ testRun(void)
 
         TEST_RESULT_VOID(iniParse(ini, content), "load ini");
 
-        TEST_RESULT_STR(strPtr(varStr(iniGet(ini, strNew("global"), strNew("compress")))), "y", "get compress");
+        TEST_RESULT_STR(strPtr(iniGet(ini, strNew("global"), strNew("compress"))), "y", "get compress");
-        TEST_RESULT_STR(strPtr(varStr(iniGet(ini, strNew("db"), strNew("pg1-path")))), "/path/to/pg", "get pg1-path");
+        TEST_RESULT_STR(strPtr(iniGet(ini, strNew("db"), strNew("pg1-path"))), "/path/to/pg", "get pg1-path");
     }
 
     FUNCTION_HARNESS_RESULT_VOID();