Skip to content

Commit

Permalink
Replace misused kvAdd() with kvPut().
Browse files Browse the repository at this point in the history 
Although kvAdd() works like kvPut() on the first call, kvPut() is more efficient when a key has a single value.

Update the comment to clarify that kvAdd() is seldom required.
  • Loading branch information
@dwsteele
dwsteele committed Apr 23, 2021
1 parent a2f02f9 commit bcc925b
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 39 deletions.
62 changes: 31 additions & 31 deletions src/command/info/info.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,47 +188,47 @@ stanzaStatus(const int code, bool backupLockHeld, Variant *stanzaInfo)

KeyValue *statusKv = kvPutKv(varKv(stanzaInfo), STANZA_KEY_STATUS_VAR);

kvAdd(statusKv, STATUS_KEY_CODE_VAR, VARINT(code));
kvPut(statusKv, STATUS_KEY_CODE_VAR, VARINT(code));

switch (code)
{
case INFO_STANZA_STATUS_CODE_OK:
kvAdd(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_OK_STR));
kvPut(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_OK_STR));
break;

case INFO_STANZA_STATUS_CODE_MISSING_STANZA_PATH:
kvAdd(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_MISSING_STANZA_PATH_STR));
kvPut(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_MISSING_STANZA_PATH_STR));
break;

case INFO_STANZA_STATUS_CODE_MISSING_STANZA_DATA:
kvAdd(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_MISSING_STANZA_DATA_STR));
kvPut(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_MISSING_STANZA_DATA_STR));
break;

case INFO_STANZA_STATUS_CODE_NO_BACKUP:
kvAdd(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_NO_BACKUP_STR));
kvPut(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_NO_BACKUP_STR));
break;

case INFO_STANZA_STATUS_CODE_MIXED:
kvAdd(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_MESSAGE_MIXED_STR));
kvPut(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_MESSAGE_MIXED_STR));
break;

case INFO_STANZA_STATUS_CODE_PG_MISMATCH:
kvAdd(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_PG_MISMATCH_STR));
kvPut(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_PG_MISMATCH_STR));
break;

case INFO_STANZA_STATUS_CODE_BACKUP_MISSING:
kvAdd(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_CODE_BACKUP_MISSING_STR));
kvPut(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_CODE_BACKUP_MISSING_STR));
break;

case INFO_STANZA_STATUS_CODE_OTHER:
kvAdd(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_OTHER_STR));
kvPut(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_OTHER_STR));
break;
}

// Construct a specific lock part
KeyValue *lockKv = kvPutKv(statusKv, STATUS_KEY_LOCK_VAR);
KeyValue *backupLockKv = kvPutKv(lockKv, STATUS_KEY_LOCK_BACKUP_VAR);
kvAdd(backupLockKv, STATUS_KEY_LOCK_BACKUP_HELD_VAR, VARBOOL(backupLockHeld));
kvPut(backupLockKv, STATUS_KEY_LOCK_BACKUP_HELD_VAR, VARBOOL(backupLockHeld));

FUNCTION_TEST_RETURN_VOID();
}
Expand All @@ -250,32 +250,32 @@ repoStanzaStatus(const int code, Variant *repoStanzaInfo, InfoRepoData *repoData

KeyValue *statusKv = kvPutKv(varKv(repoStanzaInfo), STANZA_KEY_STATUS_VAR);

kvAdd(statusKv, STATUS_KEY_CODE_VAR, VARINT(code));
kvPut(statusKv, STATUS_KEY_CODE_VAR, VARINT(code));

switch (code)
{
case INFO_STANZA_STATUS_CODE_OK:
kvAdd(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_OK_STR));
kvPut(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_OK_STR));
break;

case INFO_STANZA_STATUS_CODE_MISSING_STANZA_PATH:
kvAdd(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_MISSING_STANZA_PATH_STR));
kvPut(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_MISSING_STANZA_PATH_STR));
break;

case INFO_STANZA_STATUS_CODE_MISSING_STANZA_DATA:
kvAdd(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_MISSING_STANZA_DATA_STR));
kvPut(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_MISSING_STANZA_DATA_STR));
break;

case INFO_STANZA_STATUS_CODE_NO_BACKUP:
kvAdd(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_NO_BACKUP_STR));
kvPut(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_MESSAGE_NO_BACKUP_STR));
break;

case INFO_STANZA_STATUS_CODE_BACKUP_MISSING:
kvAdd(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_CODE_BACKUP_MISSING_STR));
kvPut(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(INFO_STANZA_STATUS_CODE_BACKUP_MISSING_STR));
break;

case INFO_STANZA_STATUS_CODE_OTHER:
kvAdd(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(repoData->error));
kvPut(statusKv, STATUS_KEY_MESSAGE_VAR, VARSTR(repoData->error));
break;
}

Expand Down Expand Up @@ -363,8 +363,8 @@ archiveDbList(
// Add empty database section to archiveInfo and then fill in database id from the backup.info
KeyValue *databaseInfo = kvPutKv(varKv(archiveInfo), KEY_DATABASE_VAR);

kvAdd(databaseInfo, DB_KEY_ID_VAR, VARUINT(pgData->id));
kvAdd(databaseInfo, KEY_REPO_KEY_VAR, VARUINT(repoKey));
kvPut(databaseInfo, DB_KEY_ID_VAR, VARUINT(pgData->id));
kvPut(databaseInfo, KEY_REPO_KEY_VAR, VARUINT(repoKey));

kvPut(varKv(archiveInfo), DB_KEY_ID_VAR, VARSTR(archiveId));
kvPut(varKv(archiveInfo), ARCHIVE_KEY_MIN_VAR, (archiveStart != NULL ? VARSTR(archiveStart) : (Variant *)NULL));
Expand Down Expand Up @@ -409,43 +409,43 @@ backupListAdd(
// archive section
KeyValue *archiveInfo = kvPutKv(varKv(backupInfo), KEY_ARCHIVE_VAR);

kvAdd(
kvPut(
archiveInfo, KEY_START_VAR,
(backupData->backupArchiveStart != NULL ? VARSTR(backupData->backupArchiveStart) : NULL));
kvAdd(
kvPut(
archiveInfo, KEY_STOP_VAR,
(backupData->backupArchiveStop != NULL ? VARSTR(backupData->backupArchiveStop) : NULL));

// backrest section
KeyValue *backrestInfo = kvPutKv(varKv(backupInfo), BACKUP_KEY_BACKREST_VAR);

kvAdd(backrestInfo, BACKREST_KEY_FORMAT_VAR, VARUINT(backupData->backrestFormat));
kvAdd(backrestInfo, BACKREST_KEY_VERSION_VAR, VARSTR(backupData->backrestVersion));
kvPut(backrestInfo, BACKREST_KEY_FORMAT_VAR, VARUINT(backupData->backrestFormat));
kvPut(backrestInfo, BACKREST_KEY_VERSION_VAR, VARSTR(backupData->backrestVersion));

// database section
KeyValue *dbInfo = kvPutKv(varKv(backupInfo), KEY_DATABASE_VAR);

kvAdd(dbInfo, DB_KEY_ID_VAR, VARUINT(backupData->backupPgId));
kvAdd(dbInfo, KEY_REPO_KEY_VAR, VARUINT(repoData->key));
kvPut(dbInfo, DB_KEY_ID_VAR, VARUINT(backupData->backupPgId));
kvPut(dbInfo, KEY_REPO_KEY_VAR, VARUINT(repoData->key));

// info section
KeyValue *infoInfo = kvPutKv(varKv(backupInfo), BACKUP_KEY_INFO_VAR);

kvAdd(infoInfo, KEY_SIZE_VAR, VARUINT64(backupData->backupInfoSize));
kvAdd(infoInfo, KEY_DELTA_VAR, VARUINT64(backupData->backupInfoSizeDelta));
kvPut(infoInfo, KEY_SIZE_VAR, VARUINT64(backupData->backupInfoSize));
kvPut(infoInfo, KEY_DELTA_VAR, VARUINT64(backupData->backupInfoSizeDelta));

// info:repository section
KeyValue *repoInfo = kvPutKv(infoInfo, INFO_KEY_REPOSITORY_VAR);

kvAdd(repoInfo, KEY_SIZE_VAR, VARUINT64(backupData->backupInfoRepoSize));
kvAdd(repoInfo, KEY_DELTA_VAR, VARUINT64(backupData->backupInfoRepoSizeDelta));
kvPut(repoInfo, KEY_SIZE_VAR, VARUINT64(backupData->backupInfoRepoSize));
kvPut(repoInfo, KEY_DELTA_VAR, VARUINT64(backupData->backupInfoRepoSizeDelta));

// timestamp section
KeyValue *timeInfo = kvPutKv(varKv(backupInfo), BACKUP_KEY_TIMESTAMP_VAR);

// time_t is generally a signed int so cast it to uint64 since it can never be negative (before 1970) in our system
kvAdd(timeInfo, KEY_START_VAR, VARUINT64((uint64_t)backupData->backupTimestampStart));
kvAdd(timeInfo, KEY_STOP_VAR, VARUINT64((uint64_t)backupData->backupTimestampStop));
kvPut(timeInfo, KEY_START_VAR, VARUINT64((uint64_t)backupData->backupTimestampStart));
kvPut(timeInfo, KEY_STOP_VAR, VARUINT64((uint64_t)backupData->backupTimestampStop));

// If a backup label was specified and this is that label, then get the data from the loaded manifest
if (backupLabel != NULL && strEq(backupData->backupLabel, backupLabel))
Expand Down
2 changes: 1 addition & 1 deletion src/common/io/filter/group.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ ioFilterGroupParamAll(const IoFilterGroup *this)
const VariantList *paramList = ioFilterParamList(filter);

KeyValue *filterParam = kvNew();
kvAdd(filterParam, VARSTR(ioFilterType(filter)), paramList ? varNewVarLst(paramList) : NULL);
kvPut(filterParam, VARSTR(ioFilterType(filter)), paramList ? varNewVarLst(paramList) : NULL);

varLstAdd(result, varNewKv(filterParam));
}
Expand Down
2 changes: 1 addition & 1 deletion src/common/stat.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ statToKv(void)
Stat *stat = lstGet(statLocalData.stat, statIdx);

KeyValue *statKv = kvPutKv(result, VARSTR(stat->key));
kvAdd(statKv, STAT_VALUE_TOTAL_VAR, VARUINT64(stat->total));
kvPut(statKv, STAT_VALUE_TOTAL_VAR, VARUINT64(stat->total));
}

FUNCTION_TEST_RETURN(result);
Expand Down
5 changes: 4 additions & 1 deletion src/common/type/keyValue.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ kvKeyList(const KeyValue *const this)
/***********************************************************************************************************************************
Functions
***********************************************************************************************************************************/
// Add value to key -- if the key does not exist then this works the same as kvPut()
// Add a value to an existing key. If the key does not exist then this works the same as kvPut(). If the key does exist then the
// value is converted to a VariantList with the existing value and the new value as list items. Subsequent calls expand the list.
//
// NOTE: this function is seldom required and kvPut() should be used when a key is expected to have a single value.
KeyValue *kvAdd(KeyValue *this, const Variant *key, const Variant *value);

// Move to a new parent mem context
Expand Down
2 changes: 1 addition & 1 deletion src/protocol/server.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ protocolServerResponse(ProtocolServer *this, const Variant *output)
KeyValue *result = kvNew();

if (output != NULL)
kvAdd(result, VARSTR(PROTOCOL_OUTPUT_STR), output);
kvPut(result, VARSTR(PROTOCOL_OUTPUT_STR), output);

ioWriteStrLine(protocolServerIoWrite(this), jsonFromKv(result));
ioWriteFlush(protocolServerIoWrite(this));
Expand Down
6 changes: 3 additions & 3 deletions test/src/module/common/typeJsonTest.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,9 @@ testRun(void)
// Embed a keyValue section to test recursion
Variant *sectionKey = varNewStr(strNew("section"));
KeyValue *sectionKv = kvPutKv(varKv(keyValue), sectionKey);
kvAdd(sectionKv, varNewStr(strNew("key1")), varNewStr(strNew("value1")));
kvAdd(sectionKv, varNewStr(strNew("key2")), (Variant *)NULL);
kvAdd(sectionKv, varNewStr(strNew("key3")), varNewStr(strNew("value2")));
kvPut(sectionKv, varNewStr(strNew("key1")), varNewStr(strNew("value1")));
kvPut(sectionKv, varNewStr(strNew("key2")), (Variant *)NULL);
kvPut(sectionKv, varNewStr(strNew("key3")), varNewStr(strNew("value2")));
kvAdd(sectionKv, varNewStr(strNew("escape")), varNewStr(strNew("\"\\/\b\n\r\t\f")));

TEST_ASSIGN(json, jsonFromVar(keyValue), "KeyValue");
Expand Down
2 changes: 1 addition & 1 deletion test/src/module/storage/remoteTest.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ testRun(void)
varLstAdd(paramList, varNewStr(strNewFmt("%s/repo/test.txt", testPath())));
varLstAdd(paramList, varNewBool(false));
varLstAdd(paramList, NULL);
varLstAdd(paramList, varNewVarLst(varLstAdd(varLstNew(), varNewKv(kvAdd(kvNew(), varNewStrZ("bogus"), NULL)))));
varLstAdd(paramList, varNewVarLst(varLstAdd(varLstNew(), varNewKv(kvPut(kvNew(), varNewStrZ("bogus"), NULL)))));

TEST_ERROR(storageRemoteOpenReadProtocol(paramList, server), AssertError, "unable to add filter 'bogus'");
}
Expand Down

0 comments on commit bcc925b

Please sign in to comment.