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

Code before:
                  server.rdb_last_load_keys_loaded, server.rdb_last_load_keys_expired);
    }
    return C_OK;

    /* Unexpected end of file is handled here calling rdbReportReadError():
     * this will in turn either abort the server in most cases, or if we are loading
     * the RDB file from a socket during initial SYNC (diskless replica mode),
     * we'll report the error to the caller, so that we can retry. */
eoferr:
    serverLog(LL_WARNING, "Short read or OOM loading DB. Unrecoverable error, aborting now.");
    rdbReportReadError("Unexpected EOF reading RDB file");
    return C_ERR;
}

/* Like rdbLoadRio() but takes a filename instead of a rio stream. The
 * filename is open for reading and a rio stream object created in order
 * to do the actual loading. Moreover the ETA displayed in the INFO
 * output is initialized and finalized.
 *
 * If you pass an 'rsi' structure initialized with RDB_SAVE_INFO_INIT, the
 * loading code will fill the information fields in the structure. */
int rdbLoad(char *filename, rdbSaveInfo *rsi, int rdbflags) {
    FILE *fp;
    rio rdb;
    int retval;
    struct stat sb;
    int rdb_fd;

    fp = fopen(filename, "r");
    if (fp == NULL) {
        if (errno == ENOENT) return RDB_NOT_EXIST;

        serverLog(LL_WARNING, "Fatal error: can't open the RDB file %s for reading: %s", filename, strerror(errno));
        return RDB_FAILED;
    }

    if (fstat(fileno(fp), &sb) == -1) sb.st_size = 0;

    startLoadingFile(sb.st_size, filename, rdbflags);
    rioInitWithFile(&rdb, fp);

    retval = rdbLoadRio(&rdb, rdbflags, rsi);

    fclose(fp);
    stopLoading(retval == C_OK);
    /* Reclaim the cache backed by rdb */
    if (retval == C_OK && !(rdbflags & RDBFLAGS_KEEP_CACHE)) {
        /* TODO: maybe we could combine the fopen and open into one in the future */
        rdb_fd = open(filename, O_RDONLY);
        if (rdb_fd >= 0) bioCreateCloseJob(rdb_fd, 0, 1);
    }

Review comment (suggestion):
This is because of the implicitly changed semantics of dbAddRDBLoad?

```C
/* This is a special version of dbAdd() that is used only when loading
 * keys from the RDB file: the key is passed as an SDS string that is
 * retained by the function (and not freed by the caller).
 *
 * Moreover this function will not abort if the key is already busy, to
 * give more control to the caller, nor will signal the key as ready
 * since it is not useful in this context.
 *
 * The function returns 1 if the key was added to the database, taking
 * ownership of the SDS string, otherwise 0 is returned, and is up to the
 * caller to free the SDS string. */
int dbAddRDBLoad(serverDb *db, sds key, robj *val) {
    int slot = getKeySlot(key);
    dictEntry *de = kvstoreDictAddRaw(db->keys, slot, key, NULL);
    if (de == NULL) return 0;
    initObjectLRUOrLFU(val);
    kvstoreDictSetVal(db->keys, slot, de, val);
    return 1;
}
```

We need to update the documentation comment for that function. It no longer retains the key. Now it makes a copy without taking ownership of the key.

Code after:
            if (!added) {
                if (rdbflags & RDBFLAGS_ALLOW_DUP) {
                    /* This flag is useful for DEBUG RELOAD special modes.
                     * When it's set we allow new keys to replace the current
                     * keys with the same name. */
                    dbSyncDelete(db, &keyobj);
                    dbAddRDBLoad(db, key, val);
                } else {
                    serverLog(LL_WARNING, "RDB has duplicated key '%s' in DB %d", key, db->id);
                    serverPanic("Duplicated key found in RDB file");
                }
            }

            /* Set the expire time if needed */
            if (expiretime != -1) {
                setExpire(NULL, db, &keyobj, expiretime);
            }

            /* Set usage information (for eviction). */
            objectSetLRUOrLFU(val, lfu_freq, lru_idle, lru_clock, 1000);

            /* call key space notification on key loaded for modules only */
            moduleNotifyKeyspaceEvent(NOTIFY_LOADED, "loaded", &keyobj, db->id);

            /* Release key (sds), dictEntry stores a copy of it in embedded data */
            sdsfree(key);
        }

        /* Loading the database more slowly is useful in order to test
         * certain edge cases. */
        if (server.key_load_delay) debugDelay(server.key_load_delay);

        /* Reset the state that is key-specified and is populated by
         * opcodes before the key, so that we start from scratch again. */
        expiretime = -1;
        lfu_freq = -1;
        lru_idle = -1;
    }
    /* Verify the checksum if RDB version is >= 5 */
    if (rdbver >= 5) {
        uint64_t cksum, expected = rdb->cksum;

        if (rioRead(rdb, &cksum, 8) == 0) goto eoferr;
        if (server.rdb_checksum && !server.skip_checksum_validation) {
            memrev64ifbe(&cksum);
            if (cksum == 0) {
                serverLog(LL_NOTICE, "RDB file was saved with checksum disabled: no check performed.");
            } else if (cksum != expected) {
                serverLog(LL_WARNING,
                          "Wrong RDB checksum expected: (%llx) but "
                          "got (%llx). Aborting now.",