From 61772d35b7d8f4bc4071c695b3cb165f7475471e Mon Sep 17 00:00:00 2001 From: Rain Valentine Date: Fri, 3 Jan 2025 22:09:29 +0000 Subject: [PATCH] make hashTypeEntry opaque and consistent naming: hash keys contain field/value pairs Signed-off-by: Rain Valentine --- src/aof.c | 4 +- src/db.c | 5 +- src/debug.c | 2 +- src/defrag.c | 13 ++-- src/module.c | 5 +- src/object.c | 10 +-- src/rdb.c | 6 +- src/server.c | 33 +++------ src/server.h | 16 ++--- src/t_hash.c | 194 ++++++++++++++++++++++++++++++++------------------- 10 files changed, 157 insertions(+), 131 deletions(-) diff --git a/src/aof.c b/src/aof.c index 531542594c..a29a7ba227 100644 --- a/src/aof.c +++ b/src/aof.c @@ -1920,7 +1920,7 @@ int rewriteSortedSetObject(rio *r, robj *key, robj *o) { /* Write either the key or the value of the currently selected item of a hash. * The 'hi' argument passes a valid hash iterator. * The 'what' filed specifies if to write a key or a value and can be - * either OBJ_HASH_KEY or OBJ_HASH_VALUE. + * either OBJ_HASH_FIELD or OBJ_HASH_VALUE. * * The function returns 0 on error, non-zero on success. */ static int rioWriteHashIteratorCursor(rio *r, hashTypeIterator *hi, int what) { @@ -1961,7 +1961,7 @@ int rewriteHashObject(rio *r, robj *key, robj *o) { } } - if (!rioWriteHashIteratorCursor(r, &hi, OBJ_HASH_KEY) || !rioWriteHashIteratorCursor(r, &hi, OBJ_HASH_VALUE)) { + if (!rioWriteHashIteratorCursor(r, &hi, OBJ_HASH_FIELD) || !rioWriteHashIteratorCursor(r, &hi, OBJ_HASH_VALUE)) { hashTypeResetIterator(&hi); return 0; } diff --git a/src/db.c b/src/db.c index 4512fc590d..f73ae02258 100644 --- a/src/db.c +++ b/src/db.c @@ -999,10 +999,9 @@ void hashtableScanCallback(void *privdata, void *entry) { key = node->ele; /* zset data is copied after filtering by key */ } else if (o->type == OBJ_HASH) { - key = (sds)hashTypeEntryGetKey(entry); + key = hashTypeEntryGetField(entry); if (!data->only_keys) { - hashTypeEntry *hash_entry = entry; - val = hash_entry->value; + val = hashTypeEntryGetValue(entry); } } else { serverPanic("Type not handled in hashtable SCAN callback."); diff --git a/src/debug.c b/src/debug.c index b444f754f9..a40e01f254 100644 --- a/src/debug.c +++ b/src/debug.c @@ -230,7 +230,7 @@ void xorObjectDigest(serverDb *db, robj *keyobj, unsigned char *digest, robj *o) sds sdsele; memset(eledigest, 0, 20); - sdsele = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_KEY); + sdsele = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_FIELD); mixDigest(eledigest, sdsele, sdslen(sdsele)); sdsfree(sdsele); sdsele = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_VALUE); diff --git a/src/defrag.c b/src/defrag.c index 2aaaf7853a..49b3918425 100644 --- a/src/defrag.c +++ b/src/defrag.c @@ -479,23 +479,18 @@ static void scanLaterSet(robj *ob, unsigned long *cursor) { *cursor = hashtableScanDefrag(ht, *cursor, activeDefragSdsHashtableCallback, NULL, activeDefragAlloc, HASHTABLE_SCAN_EMIT_REF); } -static void activeDefragHashField(void *privdata, void *element_ref) { +static void activeDefragHashTypeEntry(void *privdata, void *element_ref) { UNUSED(privdata); hashTypeEntry **entry_ref = (hashTypeEntry **)element_ref; - /* defragment field */ - hashTypeEntry *new_entry = activeDefragAlloc(*entry_ref); + hashTypeEntry *new_entry = hashTypeEntryDefrag(*entry_ref, activeDefragAlloc, activeDefragSds); if (new_entry) *entry_ref = new_entry; - - /* defragment value */ - sds new_value = activeDefragSds((*entry_ref)->value); - if (new_value) (*entry_ref)->value = new_value; } static void scanLaterHash(robj *ob, unsigned long *cursor) { if (ob->type != OBJ_HASH || ob->encoding != OBJ_ENCODING_HASHTABLE) return; hashtable *ht = ob->ptr; - *cursor = hashtableScanDefrag(ht, *cursor, activeDefragHashField, NULL, activeDefragAlloc, HASHTABLE_SCAN_EMIT_REF); + *cursor = hashtableScanDefrag(ht, *cursor, activeDefragHashTypeEntry, NULL, activeDefragAlloc, HASHTABLE_SCAN_EMIT_REF); } static void defragQuicklist(robj *ob) { @@ -540,7 +535,7 @@ static void defragHash(robj *ob) { } else { unsigned long cursor = 0; do { - cursor = hashtableScanDefrag(ht, cursor, activeDefragHashField, NULL, activeDefragAlloc, HASHTABLE_SCAN_EMIT_REF); + cursor = hashtableScanDefrag(ht, cursor, activeDefragHashTypeEntry, NULL, activeDefragAlloc, HASHTABLE_SCAN_EMIT_REF); } while (cursor != 0); } /* defrag the hashtable struct and tables */ diff --git a/src/module.c b/src/module.c index b647deff7e..f88517fcc3 100644 --- a/src/module.c +++ b/src/module.c @@ -11031,9 +11031,8 @@ static void moduleScanKeyHashtableCallback(void *privdata, void *entry) { key = node->ele; value = createStringObjectFromLongDouble(node->score, 0); } else if (o->type == OBJ_HASH) { - key = (sds)hashTypeEntryGetKey(entry); - hashTypeEntry *hash_entry = entry; - sds val = hash_entry->value; + key = hashTypeEntryGetField(entry); + sds val = hashTypeEntryGetValue(entry); value = createStringObject(val, sdslen(val)); } else { serverPanic("unexpected object type"); diff --git a/src/object.c b/src/object.c index 40f9e2aa53..843e8e4fca 100644 --- a/src/object.c +++ b/src/object.c @@ -684,11 +684,7 @@ void dismissHashObject(robj *o, size_t size_hint) { hashtableInitIterator(&iter, ht); void *next; while (hashtableNext(&iter, &next)) { - /* Only dismiss values memory since the field size - * usually is small. */ - hashTypeEntry *entry = next; - UNUSED(entry); - dismissSds(entry->value); + dismissHashTypeEntry(next); } hashtableResetIterator(&iter); } @@ -1205,9 +1201,7 @@ size_t objectComputeSize(robj *key, robj *o, size_t sample_size, int dbid) { asize = sizeof(*o) + hashtableMemUsage(ht); while (hashtableNext(&iter, &next) && samples < sample_size) { - elesize += zmalloc_usable_size(next); - hashTypeEntry *entry = next; - elesize += sdsAllocSize(entry->value); + elesize += hashTypeEntryAllocSize(next); samples++; } hashtableResetIterator(&iter); diff --git a/src/rdb.c b/src/rdb.c index 5cdbe468d1..121beb2775 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -960,8 +960,8 @@ ssize_t rdbSaveObject(rio *rdb, robj *o, robj *key, int dbid) { hashtableInitIterator(&iter, ht); void *next; while (hashtableNext(&iter, &next)) { - sds field = (sds)hashTypeEntryGetKey(next); - sds value = ((hashTypeEntry *)next)->value; + sds field = hashTypeEntryGetField(next); + sds value = hashTypeEntryGetValue(next); if ((n = rdbSaveRawString(rdb, (unsigned char *)field, sdslen(field))) == -1) { hashtableResetIterator(&iter); @@ -2166,7 +2166,7 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { } /* Add pair to hash table */ - hashTypeEntry *entry = hashTypeCreateEntry(field, value); // TODO rainval avoid extra allocation of field? + hashTypeEntry *entry = hashTypeCreateEntry(field, value); sdsfree(field); if (!hashtableAdd((hashtable *)o->ptr, entry)) { rdbReportCorruptRDB("Duplicate hash fields detected"); diff --git a/src/server.c b/src/server.c index 8d7838c3c1..a88ecf2bec 100644 --- a/src/server.c +++ b/src/server.c @@ -623,37 +623,22 @@ hashtableType subcommandSetType = {.entryGetKey = hashtableSubcommandGetKey, .keyCompare = hashtableStringKeyCaseCompare, .instant_rehashing = 1}; -/* takes ownership of value, does not take ownership of field */ -hashTypeEntry *hashTypeCreateEntry(const sds field, sds value) { - size_t field_size = sdsAllocSize(field); - void *field_data = sdsAllocPtr(field); - - size_t total_size = sizeof(hashTypeEntry) + field_size; - hashTypeEntry *hf = zmalloc(total_size); - - hf->value = value; - hf->field_offset = field - (char *)field_data; - memcpy(hf->field_data, field_data, field_size); - return hf; -} - -const void *hashTypeEntryGetKey(const void *entry) { - const hashTypeEntry *hf = entry; - return hf->field_data + hf->field_offset; +/* Hash type hash table (note that small hashes are represented with listpacks) */ +const void *hashHashtableTypeGetKey(const void *entry) { + const hashTypeEntry *hash_entry = entry; + return (const void *)hashTypeEntryGetField(hash_entry); } -void freeHashTypeEntry(void *entry) { - hashTypeEntry *hf = entry; - sdsfree(hf->value); - zfree(hf); +void hashHashtableTypeDestructor(void *entry) { + hashTypeEntry *hash_entry = entry; + freeHashTypeEntry(hash_entry); } -/* Hash type hash table (note that small hashes are represented with listpacks) */ hashtableType hashHashtableType = { .hashFunction = dictSdsHash, - .entryGetKey = hashTypeEntryGetKey, + .entryGetKey = hashHashtableTypeGetKey, .keyCompare = hashtableSdsKeyCompare, - .entryDestructor = freeHashTypeEntry, + .entryDestructor = hashHashtableTypeDestructor, }; /* Hash type hash table (note that small hashes are represented with listpacks) */ diff --git a/src/server.h b/src/server.h index 2f6ef6a5fe..476e215004 100644 --- a/src/server.h +++ b/src/server.h @@ -2661,7 +2661,7 @@ typedef struct { #include "stream.h" /* Stream data type header file. */ -#define OBJ_HASH_KEY 1 +#define OBJ_HASH_FIELD 1 #define OBJ_HASH_VALUE 2 /*----------------------------------------------------------------------------- @@ -3420,14 +3420,14 @@ robj *setTypeDup(robj *o); #define HASH_SET_TAKE_VALUE (1 << 1) #define HASH_SET_COPY 0 -typedef struct { - sds value; - unsigned char field_offset; - char field_data[]; -} hashTypeEntry; +typedef struct hashTypeEntry hashTypeEntry; hashTypeEntry *hashTypeCreateEntry(sds field, sds value); -const void *hashTypeEntryGetKey(const void *entry); -void freeHashTypeEntry(void *entry); +sds hashTypeEntryGetField(const hashTypeEntry *entry); +sds hashTypeEntryGetValue(const hashTypeEntry *entry); +size_t hashTypeEntryAllocSize(hashTypeEntry *entry); +hashTypeEntry *hashTypeEntryDefrag(hashTypeEntry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(sds)); +void dismissHashTypeEntry(hashTypeEntry *entry); +void freeHashTypeEntry(hashTypeEntry *entry); void hashTypeConvert(robj *o, int enc); void hashTypeTryConversion(robj *subject, robj **argv, int start, int end); diff --git a/src/t_hash.c b/src/t_hash.c index dbb02a1583..3af41ddddb 100644 --- a/src/t_hash.c +++ b/src/t_hash.c @@ -30,6 +30,65 @@ #include "server.h" #include +struct hashTypeEntry { + sds value; + unsigned char field_offset; + unsigned char field_data[]; +}; + +/* takes ownership of value, does not take ownership of field */ +hashTypeEntry *hashTypeCreateEntry(const sds field, sds value) { + size_t field_size = sdscopytobuffer(NULL, 0, field, NULL); + + size_t total_size = sizeof(hashTypeEntry) + field_size; + hashTypeEntry *entry = zmalloc(total_size); + + entry->value = value; + sdscopytobuffer(entry->field_data, field_size, field, &entry->field_offset); + return entry; +} + +sds hashTypeEntryGetField(const hashTypeEntry *entry) { + const unsigned char *field = entry->field_data + entry->field_offset; + return (sds)field; +} + +sds hashTypeEntryGetValue(const hashTypeEntry *entry) { + return entry->value; +} + +/* frees previous value, takes ownership of new value */ +static void hashTypeEntryReplaceValue(hashTypeEntry *entry, sds value) { + sdsfree(entry->value); + entry->value = value; +} + +size_t hashTypeEntryAllocSize(hashTypeEntry *entry) { + size_t size = zmalloc_usable_size(entry); + size += sdsAllocSize(entry->value); + return size; +} + +hashTypeEntry *hashTypeEntryDefrag(hashTypeEntry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(sds)) { + hashTypeEntry *new_entry = defragfn(entry); + if (new_entry) entry = new_entry; + + sds new_value = sdsdefragfn(entry->value); + if (new_value) entry->value = new_value; + + return entry; +} + +void dismissHashTypeEntry(hashTypeEntry *entry) { + /* Only dismiss values memory since the field size usually is small. */ + dismissSds(entry->value); +} + +void freeHashTypeEntry(hashTypeEntry *entry) { + sdsfree(entry->value); + zfree(entry); +} + /*----------------------------------------------------------------------------- * Hash type API *----------------------------------------------------------------------------*/ @@ -49,7 +108,7 @@ void hashTypeTryConversion(robj *o, robj **argv, int start, int end) { size_t new_fields = (end - start + 1) / 2; if (new_fields > server.hash_max_listpack_entries) { hashTypeConvert(o, OBJ_ENCODING_HASHTABLE); - dictExpand(o->ptr, new_fields); + hashtableExpand(o->ptr, new_fields); return; } @@ -96,10 +155,9 @@ int hashTypeGetFromListpack(robj *o, sds field, unsigned char **vstr, unsigned i * is returned. */ sds hashTypeGetFromHashTable(robj *o, sds field) { serverAssert(o->encoding == OBJ_ENCODING_HASHTABLE); - void *entry; - if (!hashtableFind(o->ptr, field, &entry)) return NULL; - hashTypeEntry *hf = entry; - return hf->value; + void *found_element; + if (!hashtableFind(o->ptr, field, &found_element)) return NULL; + return hashTypeEntryGetValue(found_element); } /* Higher level function of hashTypeGet*() that returns the hash value @@ -171,7 +229,7 @@ int hashTypeExists(robj *o, sds field) { /* Add a new field, overwrite the old with the new value if it already exists. * Return 0 on insert and 1 on update. * - * By default, the key and value SDS strings are copied if needed, so the + * By default, the field and value SDS strings are copied if needed, so the * caller retains ownership of the strings passed. However this behavior * can be effected by passing appropriate flags (possibly bitwise OR-ed): * @@ -246,9 +304,7 @@ int hashTypeSet(robj *o, sds field, sds value, int flags) { hashtableInsertAtPosition(ht, entry, &position); } else { /* exists: replace value */ - hashTypeEntry *entry = existing; - sdsfree(entry->value); - entry->value = v; + hashTypeEntryReplaceValue(existing, v); update = 1; } } else { @@ -275,7 +331,7 @@ int hashTypeDelete(robj *o, sds field) { if (fptr != NULL) { fptr = lpFind(zl, fptr, (unsigned char *)field, sdslen(field), 1); if (fptr != NULL) { - /* Delete both of the key and the value. */ + /* Delete both field and value. */ zl = lpDeleteRangeWithEntry(zl, &fptr, 2); o->ptr = zl; deleted = 1; @@ -367,7 +423,7 @@ void hashTypeCurrentFromListpack(hashTypeIterator *hi, long long *vll) { serverAssert(hi->encoding == OBJ_ENCODING_LISTPACK); - if (what & OBJ_HASH_KEY) { + if (what & OBJ_HASH_FIELD) { *vstr = lpGetValue(hi->fptr, vlen, vll); } else { *vstr = lpGetValue(hi->vptr, vlen, vll); @@ -380,12 +436,10 @@ void hashTypeCurrentFromListpack(hashTypeIterator *hi, sds hashTypeCurrentFromHashTable(hashTypeIterator *hi, int what) { serverAssert(hi->encoding == OBJ_ENCODING_HASHTABLE); - if (what & OBJ_HASH_KEY) { - const void *key = hashTypeEntryGetKey(hi->next); - return (sds)key; + if (what & OBJ_HASH_FIELD) { + return hashTypeEntryGetField(hi->next); } else { - hashTypeEntry *field = hi->next; - return field->value; + return hashTypeEntryGetValue(hi->next); } } @@ -412,7 +466,7 @@ static void hashTypeCurrentObject(hashTypeIterator *hi, int what, unsigned char } } -/* Return the key or value at the current iterator position as a new +/* Return the field or value at the current iterator position as a new * SDS string. */ sds hashTypeCurrentObjectNewSds(hashTypeIterator *hi, int what) { unsigned char *vstr; @@ -452,12 +506,12 @@ void hashTypeConvertListpack(robj *o, int enc) { hashTypeInitIterator(o, &hi); while (hashTypeNext(&hi) != C_ERR) { - sds key = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_KEY); // TODO rainval don't copy twice - here and creation of entry + sds field = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_FIELD); sds value = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_VALUE); - hashTypeEntry *field = hashTypeCreateEntry(key, value); - sdsfree(key); - if (!hashtableAdd(ht, field)) { - freeHashTypeEntry(field); + hashTypeEntry *entry = hashTypeCreateEntry(field, value); + sdsfree(field); + if (!hashtableAdd(ht, entry)) { + freeHashTypeEntry(entry); hashTypeResetIterator(&hi); /* Needed for gcc ASAN */ serverLogHexDump(LL_WARNING, "listpack with dup elements dump", o->ptr, lpBytes(o->ptr)); serverPanic("Listpack corruption detected"); @@ -507,7 +561,7 @@ robj *hashTypeDup(robj *o) { hashTypeInitIterator(o, &hi); while (hashTypeNext(&hi) != C_ERR) { /* Extract a field-value pair from an original hash object.*/ - sds field = hashTypeCurrentFromHashTable(&hi, OBJ_HASH_KEY); + sds field = hashTypeCurrentFromHashTable(&hi, OBJ_HASH_FIELD); sds value = hashTypeCurrentFromHashTable(&hi, OBJ_HASH_VALUE); /* Add a field-value pair to a new hash object. */ @@ -538,23 +592,24 @@ void hashReplyFromListpackEntry(client *c, listpackEntry *e) { } /* Return random element from a non empty hash. - * 'key' and 'val' will be set to hold the element. + * 'field' and 'val' will be set to hold the element. * The memory in them is not to be freed or modified by the caller. * 'val' can be NULL in which case it's not extracted. */ -static void hashTypeRandomElement(robj *hashobj, unsigned long hashsize, listpackEntry *key, listpackEntry *val) { +static void hashTypeRandomElement(robj *hashobj, unsigned long hashsize, listpackEntry *field, listpackEntry *val) { if (hashobj->encoding == OBJ_ENCODING_HASHTABLE) { void *entry; hashtableFairRandomEntry(hashobj->ptr, &entry); - const unsigned char *s = hashTypeEntryGetKey(entry); - key->sval = (unsigned char *)s; - key->slen = sdslen((sds)s); + sds sds_field = hashTypeEntryGetField(entry); + field->sval = (unsigned char *)sds_field; + field->slen = sdslen(sds_field); if (val) { - hashTypeEntry *field = entry; - val->sval = (unsigned char *)field->value; - val->slen = sdslen(field->value); + hashTypeEntry *hash_entry = entry; + sds sds_val = hashTypeEntryGetValue(hash_entry); + val->sval = (unsigned char *)sds_val; + val->slen = sdslen(sds_val); } } else if (hashobj->encoding == OBJ_ENCODING_LISTPACK) { - lpRandomPair(hashobj->ptr, hashsize, key, val); + lpRandomPair(hashobj->ptr, hashsize, field, val); } else { serverPanic("Unknown hash encoding"); } @@ -804,15 +859,15 @@ void genericHgetallCommand(client *c, int flags) { hashTypeIterator hi; int length, count = 0; - robj *emptyResp = (flags & OBJ_HASH_KEY && flags & OBJ_HASH_VALUE) ? shared.emptymap[c->resp] : shared.emptyarray; + robj *emptyResp = (flags & OBJ_HASH_FIELD && flags & OBJ_HASH_VALUE) ? shared.emptymap[c->resp] : shared.emptyarray; if ((o = lookupKeyReadOrReply(c, c->argv[1], emptyResp)) == NULL || checkType(c, o, OBJ_HASH)) return; writePreparedClient *wpc = prepareClientForFutureWrites(c); if (!wpc) return; - /* We return a map if the user requested keys and values, like in the + /* We return a map if the user requested fields and values, like in the * HGETALL case. Otherwise to use a flat array makes more sense. */ length = hashTypeLength(o); - if (flags & OBJ_HASH_KEY && flags & OBJ_HASH_VALUE) { + if (flags & OBJ_HASH_FIELD && flags & OBJ_HASH_VALUE) { addWritePreparedReplyMapLen(wpc, length); } else { addWritePreparedReplyArrayLen(wpc, length); @@ -820,8 +875,8 @@ void genericHgetallCommand(client *c, int flags) { hashTypeInitIterator(o, &hi); while (hashTypeNext(&hi) != C_ERR) { - if (flags & OBJ_HASH_KEY) { - addHashIteratorCursorToReply(wpc, &hi, OBJ_HASH_KEY); + if (flags & OBJ_HASH_FIELD) { + addHashIteratorCursorToReply(wpc, &hi, OBJ_HASH_FIELD); count++; } if (flags & OBJ_HASH_VALUE) { @@ -833,12 +888,12 @@ void genericHgetallCommand(client *c, int flags) { hashTypeResetIterator(&hi); /* Make sure we returned the right number of elements. */ - if (flags & OBJ_HASH_KEY && flags & OBJ_HASH_VALUE) count /= 2; + if (flags & OBJ_HASH_FIELD && flags & OBJ_HASH_VALUE) count /= 2; serverAssert(count == length); } void hkeysCommand(client *c) { - genericHgetallCommand(c, OBJ_HASH_KEY); + genericHgetallCommand(c, OBJ_HASH_FIELD); } void hvalsCommand(client *c) { @@ -846,7 +901,7 @@ void hvalsCommand(client *c) { } void hgetallCommand(client *c) { - genericHgetallCommand(c, OBJ_HASH_KEY | OBJ_HASH_VALUE); + genericHgetallCommand(c, OBJ_HASH_FIELD | OBJ_HASH_VALUE); } void hexistsCommand(client *c) { @@ -865,14 +920,14 @@ void hscanCommand(client *c) { scanGenericCommand(c, o, cursor); } -static void hrandfieldReplyWithListpack(writePreparedClient *wpc, unsigned int count, listpackEntry *keys, listpackEntry *vals) { +static void hrandfieldReplyWithListpack(writePreparedClient *wpc, unsigned int count, listpackEntry *fields, listpackEntry *vals) { client *c = (client *)wpc; for (unsigned long i = 0; i < count; i++) { if (vals && c->resp > 2) addWritePreparedReplyArrayLen(wpc, 2); - if (keys[i].sval) - addWritePreparedReplyBulkCBuffer(wpc, keys[i].sval, keys[i].slen); + if (fields[i].sval) + addWritePreparedReplyBulkCBuffer(wpc, fields[i].sval, fields[i].slen); else - addWritePreparedReplyBulkLongLong(wpc, keys[i].lval); + addWritePreparedReplyBulkLongLong(wpc, fields[i].lval); if (vals) { if (vals[i].sval) addWritePreparedReplyBulkCBuffer(wpc, vals[i].sval, vals[i].slen); @@ -929,29 +984,28 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) { while (count--) { void *entry; hashtableFairRandomEntry(hash->ptr, &entry); - hashTypeEntry *field = entry; - sds key = (sds)hashTypeEntryGetKey(entry); - sds value = field->value; + sds field = hashTypeEntryGetField(entry); + sds value = hashTypeEntryGetValue(entry); if (withvalues && c->resp > 2) addWritePreparedReplyArrayLen(wpc, 2); - addWritePreparedReplyBulkCBuffer(wpc, key, sdslen(key)); + addWritePreparedReplyBulkCBuffer(wpc, field, sdslen(field)); if (withvalues) addWritePreparedReplyBulkCBuffer(wpc, value, sdslen(value)); if (c->flag.close_asap) break; } } else if (hash->encoding == OBJ_ENCODING_LISTPACK) { - listpackEntry *keys, *vals = NULL; + listpackEntry *fields, *vals = NULL; unsigned long limit, sample_count; limit = count > HRANDFIELD_RANDOM_SAMPLE_LIMIT ? HRANDFIELD_RANDOM_SAMPLE_LIMIT : count; - keys = zmalloc(sizeof(listpackEntry) * limit); + fields = zmalloc(sizeof(listpackEntry) * limit); if (withvalues) vals = zmalloc(sizeof(listpackEntry) * limit); while (count) { sample_count = count > limit ? limit : count; count -= sample_count; - lpRandomPairs(hash->ptr, sample_count, keys, vals); - hrandfieldReplyWithListpack(wpc, sample_count, keys, vals); + lpRandomPairs(hash->ptr, sample_count, fields, vals); + hrandfieldReplyWithListpack(wpc, sample_count, fields, vals); if (c->flag.close_asap) break; } - zfree(keys); + zfree(fields); zfree(vals); } return; @@ -972,7 +1026,7 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) { hashTypeInitIterator(hash, &hi); while (hashTypeNext(&hi) != C_ERR) { if (withvalues && c->resp > 2) addWritePreparedReplyArrayLen(wpc, 2); - addHashIteratorCursorToReply(wpc, &hi, OBJ_HASH_KEY); + addHashIteratorCursorToReply(wpc, &hi, OBJ_HASH_FIELD); if (withvalues) addHashIteratorCursorToReply(wpc, &hi, OBJ_HASH_VALUE); } hashTypeResetIterator(&hi); @@ -988,12 +1042,12 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) { * And it is inefficient to repeatedly pick one random element from a * listpack in CASE 4. So we use this instead. */ if (hash->encoding == OBJ_ENCODING_LISTPACK) { - listpackEntry *keys, *vals = NULL; - keys = zmalloc(sizeof(listpackEntry) * count); + listpackEntry *fields, *vals = NULL; + fields = zmalloc(sizeof(listpackEntry) * count); if (withvalues) vals = zmalloc(sizeof(listpackEntry) * count); - serverAssert(lpRandomPairsUnique(hash->ptr, count, keys, vals) == count); - hrandfieldReplyWithListpack(wpc, count, keys, vals); - zfree(keys); + serverAssert(lpRandomPairsUnique(hash->ptr, count, fields, vals) == count); + hrandfieldReplyWithListpack(wpc, count, fields, vals); + zfree(fields); zfree(vals); return; } @@ -1017,11 +1071,11 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) { /* Add all the elements into the temporary dictionary. */ while ((hashTypeNext(&hi)) != C_ERR) { int ret = DICT_ERR; - sds key, value = NULL; + sds field, value = NULL; - key = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_KEY); + field = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_FIELD); if (withvalues) value = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_VALUE); - ret = dictAdd(d, key, value); + ret = dictAdd(d, field, value); serverAssert(ret == DICT_OK); } @@ -1044,10 +1098,10 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) { dictEntry *de; di = dictGetIterator(d); while ((de = dictNext(di)) != NULL) { - sds key = dictGetKey(de); + sds field = dictGetKey(de); sds value = dictGetVal(de); if (withvalues && c->resp > 2) addWritePreparedReplyArrayLen(wpc, 2); - addWritePreparedReplyBulkSds(wpc, key); + addWritePreparedReplyBulkSds(wpc, field); if (withvalues) addWritePreparedReplyBulkSds(wpc, value); } @@ -1062,25 +1116,25 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) { else { /* Hashtable encoding (generic implementation) */ unsigned long added = 0; - listpackEntry key, value; + listpackEntry field, value; dict *d = dictCreate(&hashDictType); dictExpand(d, count); while (added < count) { - hashTypeRandomElement(hash, size, &key, withvalues ? &value : NULL); + hashTypeRandomElement(hash, size, &field, withvalues ? &value : NULL); /* Try to add the object to the dictionary. If it already exists * free it, otherwise increment the number of objects we have * in the result dictionary. */ - sds skey = hashSdsFromListpackEntry(&key); - if (dictAdd(d, skey, NULL) != DICT_OK) { - sdsfree(skey); + sds sfield = hashSdsFromListpackEntry(&field); + if (dictAdd(d, sfield, NULL) != DICT_OK) { + sdsfree(sfield); continue; } added++; /* We can reply right away, so that we don't need to store the value in the dict. */ if (withvalues && c->resp > 2) addWritePreparedReplyArrayLen(wpc, 2); - hashReplyFromListpackEntry(c, &key); + hashReplyFromListpackEntry(c, &field); if (withvalues) hashReplyFromListpackEntry(c, &value); }