varNewKv() accepts a KeyValue object rather than creating one.

This allows for more flexibility about when the Variant is created.
pgbackrest · Apr 22, 2019 · 47491e3 · 47491e3
1 parent fdf19e5
commit 47491e3
Show file tree

Hide file tree

Showing 14 changed files with 40 additions and 29 deletions.
diff --git a/doc/xml/release.xml b/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><code>varNewKv()</code> accepts a <code>KeyValue</code> object rather than creating one.</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>

diff --git a/src/command/info/info.c b/src/command/info/info.c
@@ -119,7 +119,7 @@ archiveDbList(const String *stanza, const InfoPgData *pgData, VariantList *archi
     String *archivePath = strNewFmt(STORAGE_PATH_ARCHIVE "/%s/%s", strPtr(stanza), strPtr(archiveId));
     String *archiveStart = NULL;
     String *archiveStop = NULL;
-    Variant *archiveInfo = varNewKv();
+    Variant *archiveInfo = varNewKv(kvNew());
 
     // Get a list of WAL directories in the archive repo from oldest to newest, if any exist
     StringList *walDir = storageListP(storageRepo(), archivePath, .expression = WAL_SEGMENT_DIR_REGEXP_STR);
@@ -206,7 +206,7 @@ backupList(VariantList *backupSection, InfoBackup *info)
         // Get the backup data
         InfoBackupData backupData = infoBackupData(info, keyIdx);
 
-        Variant *backupInfo = varNewKv();
+        Variant *backupInfo = varNewKv(kvNew());
 
         // main keys
         kvPut(varKv(backupInfo), BACKUP_KEY_LABEL_VAR, VARSTR(backupData.backupLabel));
@@ -297,7 +297,7 @@ stanzaInfoList(const String *stanza, StringList *stanzaList)
         }
 
         // Create the stanzaInfo and section variables
-        Variant *stanzaInfo = varNewKv();
+        Variant *stanzaInfo = varNewKv(kvNew());
         VariantList *dbSection = varLstNew();
         VariantList *backupSection = varLstNew();
         VariantList *archiveSection = varLstNew();
@@ -345,7 +345,7 @@ stanzaInfoList(const String *stanza, StringList *stanzaList)
             for (unsigned int pgIdx = infoPgDataTotal(infoBackupPg(info)) - 1; (int)pgIdx >= 0; pgIdx--)
             {
                 InfoPgData pgData = infoPgData(infoBackupPg(info), pgIdx);
-                Variant *pgInfo = varNewKv();
+                Variant *pgInfo = varNewKv(kvNew());
 
                 kvPut(varKv(pgInfo), DB_KEY_ID_VAR, VARUINT(pgData.id));
                 kvPut(varKv(pgInfo), DB_KEY_SYSTEM_ID_VAR, VARUINT64(pgData.systemId));
@@ -386,7 +386,7 @@ stanzaInfoList(const String *stanza, StringList *stanzaList)
     // If looking for a specific stanza and it was not found, set minimum info and the status
     if (stanza != NULL && !stanzaFound)
     {
-        Variant *stanzaInfo = varNewKv();
+        Variant *stanzaInfo = varNewKv(kvNew());
 
         kvPut(varKv(stanzaInfo), STANZA_KEY_NAME_VAR, VARSTR(stanza));
 

diff --git a/src/common/type/json.c b/src/common/type/json.c
@@ -266,7 +266,7 @@ jsonToVarInternal(const char *json, unsigned int *jsonPos)
             MEM_CONTEXT_TEMP_BEGIN()
             {
                 memContextSwitch(MEM_CONTEXT_OLD());
-                result = varNewKv();
+                result = varNewKv(kvNew());
                 memContextSwitch(MEM_CONTEXT_TEMP());
 
                 // Move position to the first key/value in the object

diff --git a/src/common/type/keyValue.c b/src/common/type/keyValue.c
@@ -293,10 +293,8 @@ kvPutKv(KeyValue *this, const Variant *key)
 
     MEM_CONTEXT_BEGIN(this->memContext)
     {
-        Variant *keyValue = varNewKv();
-        result = varKv(keyValue);
-
-        kvPutInternal(this, key, keyValue);
+        result = kvNew();
+        kvPutInternal(this, key, varNewKv(result));
     }
     MEM_CONTEXT_END();
 

diff --git a/src/common/type/variant.c b/src/common/type/variant.c
@@ -967,17 +967,23 @@ varUInt64Force(const Variant *this)
 
 /***********************************************************************************************************************************
 New key/value variant
+
+Note that the kv is not duped because it this a heavy-weight operation.  It is merely moved into the same MemContext as the Variant.
 ***********************************************************************************************************************************/
 Variant *
-varNewKv(void)
+varNewKv(KeyValue *data)
 {
-    FUNCTION_TEST_VOID();
+    FUNCTION_TEST_BEGIN();
+        FUNCTION_TEST_PARAM(KEY_VALUE, data);
+    FUNCTION_TEST_END();
 
     // Allocate memory for the variant and set the type and data
     VariantKeyValue *this = memNew(sizeof(VariantKeyValue));
     this->memContext = memContextCurrent();
     this->type = varTypeKeyValue;
-    this->data = kvNew();
+
+    if (data != NULL)
+        this->data = kvMove(data, memContextCurrent());
 
     FUNCTION_TEST_RETURN((Variant *)this);
 }

diff --git a/src/common/type/variant.h b/src/common/type/variant.h
@@ -64,7 +64,7 @@ Variant *varNewInt64(int64_t data);
 int64_t varInt64(const Variant *this);
 int64_t varInt64Force(const Variant *this);
 
-Variant *varNewKv(void);
+Variant *varNewKv(KeyValue *data);
 KeyValue *varKv(const Variant *this);
 
 Variant *varNewStr(const String *data);

diff --git a/src/config/parse.c b/src/config/parse.c
@@ -964,7 +964,7 @@ configParse(unsigned int argListSize, const char *argList[], bool resetLogLevel)
                         }
                         else if (optionDefType == cfgDefOptTypeHash)
                         {
-                            Variant *value = varNewKv();
+                            Variant *value = varNewKv(kvNew());
                             KeyValue *keyValue = varKv(value);
 
                             for (unsigned int listIdx = 0; listIdx < strLstSize(parseOption->valueList); listIdx++)

diff --git a/src/perl/config.c b/src/perl/config.c
@@ -28,7 +28,7 @@ perlOptionJson(void)
             if (!cfgOptionValid(optionId))
                 continue;
 
-            Variant *optionVar = varNewKv();
+            Variant *optionVar = varNewKv(kvNew());
 
             // Add valid
             kvPut(varKv(optionVar), VARSTRDEF("valid"), BOOL_TRUE_VAR);
@@ -83,7 +83,7 @@ perlOptionJson(void)
 
                     case cfgDefOptTypeHash:
                     {
-                        valueVar = varNewKv();
+                        valueVar = varNewKv(kvNew());
 
                         const KeyValue *valueKv = cfgOptionKv(optionId);
                         const VariantList *keyList = kvKeyList(valueKv);
@@ -96,7 +96,7 @@ perlOptionJson(void)
 
                     case cfgDefOptTypeList:
                     {
-                        valueVar = varNewKv();
+                        valueVar = varNewKv(kvNew());
 
                         const VariantList *valueList = cfgOptionLst(optionId);
 

diff --git a/test/src/module/command/commandTest.c b/test/src/module/command/commandTest.c
@@ -70,7 +70,7 @@ testRun(void)
         cfgOptionSet(cfgOptDbInclude, cfgSourceParam, varNewVarLst(varLstNewStrLst(list)));
 
         cfgOptionValidSet(cfgOptRecoveryOption, true);
-        Variant *recoveryVar = varNewKv();
+        Variant *recoveryVar = varNewKv(kvNew());
         KeyValue *recoveryKv = varKv(recoveryVar);
         kvPut(recoveryKv, varNewStr(strNew("standby_mode")), varNewStr(strNew("on")));
         kvPut(recoveryKv, varNewStr(strNew("primary_conn_info")), varNewStr(strNew("blah")));

diff --git a/test/src/module/command/infoTest.c b/test/src/module/command/infoTest.c
@@ -787,7 +787,7 @@ testRun(void)
         // These tests cover branches not covered in other tests
         KeyValue *stanzaInfo = kvNew();
         VariantList *dbSection = varLstNew();
-        Variant *pgInfo = varNewKv();
+        Variant *pgInfo = varNewKv(kvNew());
         kvPut(varKv(pgInfo), DB_KEY_ID_VAR, varNewUInt(1));
         kvPut(varKv(pgInfo), DB_KEY_SYSTEM_ID_VAR, varNewUInt64(6625633699176220261));
         kvPut(varKv(pgInfo), DB_KEY_VERSION_VAR, VARSTR(pgVersionToStr(90500)));
@@ -798,7 +798,7 @@ testRun(void)
         kvPut(stanzaInfo, STANZA_KEY_DB_VAR, varNewVarLst(dbSection));
 
         VariantList *backupSection = varLstNew();
-        Variant *backupInfo = varNewKv();
+        Variant *backupInfo = varNewKv(kvNew());
 
         kvPut(varKv(backupInfo), BACKUP_KEY_LABEL_VAR, VARSTRDEF("20181119-152138F"));
         kvPut(varKv(backupInfo), BACKUP_KEY_TYPE_VAR, VARSTRDEF("full"));

diff --git a/test/src/module/common/typeJsonTest.c b/test/src/module/common/typeJsonTest.c
@@ -100,7 +100,7 @@ testRun(void)
         kvPut(keyValue, varNewStrZ("checknull"), (Variant *)NULL);
 
         VariantList *dbList = varLstNew();
-        Variant *dbInfo = varNewKv();
+        Variant *dbInfo = varNewKv(kvNew());
         kvPut(varKv(dbInfo), varNewStr(strNew("id")), varNewInt(1));
         kvPut(varKv(dbInfo), varNewStr(strNew("version")),  varNewStr(strNew("9.4")));
         varLstAdd(dbList, dbInfo);
@@ -153,7 +153,7 @@ testRun(void)
         String *json = NULL;
         Variant *keyValue = NULL;
 
-        TEST_ASSIGN(keyValue, varNewKv(), "build new kv");
+        TEST_ASSIGN(keyValue, varNewKv(kvNew()), "build new kv");
         kvPut(varKv(keyValue), varNewStrZ("backup-info-size-delta"), varNewInt(1982702));
         kvPut(varKv(keyValue), varNewStrZ("backup-prior"), varNewStrZ("20161219-212741F_20161219-212803I"));
 

diff --git a/test/src/module/common/typeVariantTest.c b/test/src/module/common/typeVariantTest.c
@@ -265,7 +265,10 @@ testRun(void)
         // -------------------------------------------------------------------------------------------------------------------------
         Variant *keyValue = NULL;
 
-        TEST_ASSIGN(keyValue, varNewKv(), "new");
+        TEST_ASSIGN(keyValue, varNewKv(NULL), "new null");
+        TEST_RESULT_PTR(varKv(keyValue), NULL, "    kv is null");
+
+        TEST_ASSIGN(keyValue, varNewKv(kvNew()), "new");
         TEST_RESULT_PTR(kvPut(varKv(keyValue), VARINT(44), VARINT(55)), varKv(keyValue), "    put int/int");
         TEST_RESULT_INT(varInt(kvGet(varKv(keyValue), VARINT(44))), 55, "    get int/int");
         TEST_RESULT_PTR(varKv(NULL), NULL, "get null kv");
@@ -280,7 +283,7 @@ testRun(void)
         varFree(keyValueDup);
 
         // -------------------------------------------------------------------------------------------------------------------------
-        TEST_ERROR(varEq(varNewKv(), varNewKv()), AssertError, "unable to test equality for KeyValue");
+        TEST_ERROR(varEq(varNewKv(kvNew()), varNewKv(kvNew())), AssertError, "unable to test equality for KeyValue");
     }
 
     // *****************************************************************************************************************************
@@ -326,7 +329,7 @@ testRun(void)
         TEST_RESULT_STR(strPtr(varStrForce(varNewBool(false))), "false", "force bool to string");
         TEST_RESULT_STR(strPtr(varStrForce(VARUINT64(18446744073709551615U))), "18446744073709551615", "force uint64 to string");
 
-        TEST_ERROR(varStrForce(varNewKv()), FormatError, "unable to force KeyValue to String");
+        TEST_ERROR(varStrForce(varNewKv(kvNew())), FormatError, "unable to force KeyValue to String");
 
         // -------------------------------------------------------------------------------------------------------------------------
         string = varNewStrZ("not-an-int");
@@ -395,7 +398,7 @@ testRun(void)
     {
         TEST_RESULT_STR(strPtr(varToLog(varNewStrZ("testme"))), "{\"testme\"}", "format String");
         TEST_RESULT_STR(strPtr(varToLog(varNewBool(false))), "{false}", "format bool");
-        TEST_RESULT_STR(strPtr(varToLog(varNewKv())), "{KeyValue}", "format KeyValue");
+        TEST_RESULT_STR(strPtr(varToLog(varNewKv(kvNew()))), "{KeyValue}", "format KeyValue");
         TEST_RESULT_STR(strPtr(varToLog(varNewVarLst(varLstNew()))), "{VariantList}", "format VariantList");
         TEST_RESULT_STR(strPtr(varToLog(NULL)), "null", "format null");
     }

diff --git a/test/src/module/config/configTest.c b/test/src/module/config/configTest.c
@@ -162,7 +162,7 @@ testRun(void)
         TEST_ERROR(
             cfgOptionSet(cfgOptRecoveryOption, cfgSourceParam, varNewDbl(1.1)), AssertError,
             "option 'recovery-option' must be set with KeyValue variant");
-        TEST_RESULT_VOID(cfgOptionSet(cfgOptRecoveryOption, cfgSourceConfig, varNewKv()), "set recovery-option");
+        TEST_RESULT_VOID(cfgOptionSet(cfgOptRecoveryOption, cfgSourceConfig, varNewKv(kvNew())), "set recovery-option");
         TEST_RESULT_INT(varLstSize(kvKeyList(cfgOptionKv(cfgOptRecoveryOption))), 0, "recovery-option is set");
         TEST_ERROR(
             cfgOptionLst(cfgOptRecoveryOption), AssertError,

diff --git a/test/src/module/perl/configTest.c b/test/src/module/perl/configTest.c
@@ -86,7 +86,7 @@ testRun(void)
 
         cfgOptionValidSet(cfgOptRecoveryOption, true);
 
-        Variant *recoveryVar = varNewKv();
+        Variant *recoveryVar = varNewKv(kvNew());
         KeyValue *recoveryKv = varKv(recoveryVar);
         kvPut(recoveryKv, varNewStr(strNew("standby_mode")), varNewStr(strNew("on")));
         kvPut(recoveryKv, varNewStr(strNew("primary_conn_info")), varNewStr(strNew("blah")));