Repository: valkey-io/valkey
File: src/rdb.c (C)

Code before:
                rdbReportCorruptRDB("Duplicate zset fields detected");
                decrRefCount(o);
                /* no need to free 'sdsele', will be released by zslFree together with 'o' */
                return NULL;
            }
        }

        /* Convert *after* loading, since sorted sets are not stored ordered. */
        if (zsetLength(o) <= server.zset_max_listpack_entries && maxelelen <= server.zset_max_listpack_value &&
            lpSafeToAdd(NULL, totelelen)) {
            zsetConvert(o, OBJ_ENCODING_LISTPACK);
        }
    } else if (rdbtype == RDB_TYPE_HASH) {
        uint64_t len;
        sds field, value;
        dict *dupSearchDict = NULL;

        len = rdbLoadLen(rdb, NULL);
        if (len == RDB_LENERR) return NULL;
        if (len == 0) goto emptykey;

        o = createHashObject();

        /* Too many entries? Use a hash table right from the start. */
        if (len > server.hash_max_listpack_entries)
            hashTypeEnsureHashtableEncoded(o);
        else if (deep_integrity_validation) {
            /* In this mode, we need to guarantee that the server won't crash
             * later when the ziplist is converted to a hashtable.
             * Create a set (dict with no values) to for a dup search.
             * We can dismiss it as soon as we convert the ziplist to a hash. */
            dupSearchDict = dictCreate(&hashDictType);
        }


        /* Load every field and value into the ziplist */
        while (o->encoding == OBJ_ENCODING_LISTPACK && len > 0) {
            len--;
            /* Load raw strings */
            if ((field = rdbGenericLoadStringObject(rdb, RDB_LOAD_SDS, NULL)) == NULL) {
                decrRefCount(o);
                if (dupSearchDict) dictRelease(dupSearchDict);
                return NULL;
            }
            if ((value = rdbGenericLoadStringObject(rdb, RDB_LOAD_SDS, NULL)) == NULL) {
                sdsfree(field);
                decrRefCount(o);
                if (dupSearchDict) dictRelease(dupSearchDict);
                return NULL;
            }


Review comment (refactor):
(This and many similar lines.)

I don't think we should do this refactoring for two reasons:

1. We want to avoid unnecessary changes. More touched lines means unnecessary risk of merge conflicts and harder to follow changes using git history and git blame.
2. Just because we currently only convert from listpack to hashtable, it doesn't mean we never will do any other conversion. We might want to convert back to listpack if the hashtable is small and we detect it during some iteration over it.

Code after:
            if (sdslen(sdsele) > maxelelen) maxelelen = sdslen(sdsele);
            totelelen += sdslen(sdsele);

            znode = zslInsert(zs->zsl, score, sdsele);
            if (!hashtableAdd(zs->ht, znode)) {
                rdbReportCorruptRDB("Duplicate zset fields detected");
                decrRefCount(o);
                /* no need to free 'sdsele', will be released by zslFree together with 'o' */
                return NULL;
            }
        }

        /* Convert *after* loading, since sorted sets are not stored ordered. */
        if (zsetLength(o) <= server.zset_max_listpack_entries && maxelelen <= server.zset_max_listpack_value &&
            lpSafeToAdd(NULL, totelelen)) {
            zsetConvert(o, OBJ_ENCODING_LISTPACK);
        }
    } else if (rdbtype == RDB_TYPE_HASH) {
        uint64_t len;
        sds field, value;
        dict *dupSearchDict = NULL;

        len = rdbLoadLen(rdb, NULL);
        if (len == RDB_LENERR) return NULL;
        if (len == 0) goto emptykey;

        o = createHashObject();

        /* Too many entries? Use a hash table right from the start. */
        if (len > server.hash_max_listpack_entries)
            hashTypeConvert(o, OBJ_ENCODING_HASHTABLE);
        else if (deep_integrity_validation) {
            /* In this mode, we need to guarantee that the server won't crash
             * later when the ziplist is converted to a hashtable.
             * Create a set (dict with no values) to for a dup search.
             * We can dismiss it as soon as we convert the ziplist to a hash. */
            dupSearchDict = dictCreate(&hashDictType);
        }


        /* Load every field and value into the ziplist */
        while (o->encoding == OBJ_ENCODING_LISTPACK && len > 0) {
            len--;
            /* Load raw strings */
            if ((field = rdbGenericLoadStringObject(rdb, RDB_LOAD_SDS, NULL)) == NULL) {
                decrRefCount(o);
                if (dupSearchDict) dictRelease(dupSearchDict);
                return NULL;
            }
            if ((value = rdbGenericLoadStringObject(rdb, RDB_LOAD_SDS, NULL)) == NULL) {
                sdsfree(field);