From 4505eb18213c8da31c6dd39ba7cd36d3d01141a5 Mon Sep 17 00:00:00 2001
From: Binbin <binloveplay1314@qq.com>
Date: Thu, 4 Aug 2022 15:47:37 +0800
Subject: [PATCH] errno cleanup around rdbLoad (#11042)

This is an addition to #11039, which cleans up rdbLoad* related errno. Remove the
errno print from the outer message (may be invalid since errno may have been overwritten).

Our aim should be the code that detects the error and knows which system call
triggered it, is the one to print errno, and not the code way up above (in some cases
a result of a logical error and not a system one).

Remove the code to update errno in rdbLoadRioWithLoadingCtx, signature check
and the rdb version check, in these cases, we do print the error message.
The caller dose not have the specific logic for handling EINVAL.

Small fix around rdb-preamble AOF: A truncated RDB is considered a failure,
not handled the same as a truncated AOF file.
---
 src/aof.c         | 3 ++-
 src/debug.c       | 2 +-
 src/rdb.c         | 9 ++++-----
 src/replication.c | 4 ++--
 src/server.c      | 2 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/aof.c b/src/aof.c
index 482ec786d..1f0b824f1 100644
--- a/src/aof.c
+++ b/src/aof.c
@@ -1432,7 +1432,8 @@ int loadSingleAppendOnlyFile(char *filename) {
             else
                 serverLog(LL_WARNING, "Error reading the RDB base file %s, AOF loading aborted", filename);
 
-            goto readerr;
+            ret = AOF_FAILED;
+            goto cleanup;
         } else {
             loadingAbsProgress(ftello(fp));
             last_progress_report_size = ftello(fp);
diff --git a/src/debug.c b/src/debug.c
index 33e4a9ac3..dac7716ac 100644
--- a/src/debug.c
+++ b/src/debug.c
@@ -568,7 +568,7 @@ NULL
         int ret = rdbLoad(server.rdb_filename,NULL,flags);
         unprotectClient(c);
         if (ret != RDB_OK) {
-            addReplyError(c,"Error trying to load the RDB dump");
+            addReplyError(c,"Error trying to load the RDB dump, check server logs.");
             return;
         }
         serverLog(LL_WARNING,"DB reloaded by DEBUG RELOAD");
diff --git a/src/rdb.c b/src/rdb.c
index fea5f4121..993a123c0 100644
--- a/src/rdb.c
+++ b/src/rdb.c
@@ -2889,7 +2889,7 @@ int rdbLoadRio(rio *rdb, int rdbflags, rdbSaveInfo *rsi) {
 
 
 /* Load an RDB file from the rio stream 'rdb'. On success C_OK is returned,
- * otherwise C_ERR is returned and 'errno' is set accordingly. 
+ * otherwise C_ERR is returned.
  * The rdb_loading_ctx argument holds objects to which the rdb will be loaded to,
  * currently it only allow to set db object and functionLibCtx to which the data
  * will be loaded (in the future it might contains more such objects). */
@@ -2907,13 +2907,11 @@ int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadin
     buf[9] = '\0';
     if (memcmp(buf,"REDIS",5) != 0) {
         serverLog(LL_WARNING,"Wrong signature trying to load DB from file");
-        errno = EINVAL;
         return C_ERR;
     }
     rdbver = atoi(buf+5);
     if (rdbver < 1 || rdbver > RDB_VERSION) {
         serverLog(LL_WARNING,"Can't handle RDB format version %d",rdbver);
-        errno = EINVAL;
         return C_ERR;
     }
 
@@ -3254,9 +3252,10 @@ int rdbLoad(char *filename, rdbSaveInfo *rsi, int rdbflags) {
 
     fp = fopen(filename, "r");
     if (fp == NULL) {
-        retval = (errno == ENOENT) ? RDB_NOT_EXIST : RDB_FAILED;
+        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 retval;
+        return RDB_FAILED;
     }
 
     if (fstat(fileno(fp), &sb) == -1)
diff --git a/src/replication.c b/src/replication.c
index 65ae27069..faf159d7d 100644
--- a/src/replication.c
+++ b/src/replication.c
@@ -2055,7 +2055,7 @@ void readSyncBulkPayload(connection *conn) {
             /* RDB loading failed. */
             serverLog(LL_WARNING,
                       "Failed trying to load the MASTER synchronization DB "
-                      "from socket: %s", strerror(errno));
+                      "from socket, check server logs.");
             loadingFailed = 1;
         } else if (usemark) {
             /* Verify the end mark is correct. */
@@ -2164,7 +2164,7 @@ void readSyncBulkPayload(connection *conn) {
         if (rdbLoad(server.rdb_filename,&rsi,RDBFLAGS_REPLICATION) != RDB_OK) {
             serverLog(LL_WARNING,
                 "Failed trying to load the MASTER synchronization "
-                "DB from disk: %s", strerror(errno));
+                "DB from disk, check server logs.");
             cancelReplicationHandshake(1);
             if (server.rdb_del_sync_files && allPersistenceDisabled()) {
                 serverLog(LL_NOTICE,"Removing the RDB file obtained from "
diff --git a/src/server.c b/src/server.c
index 27f2be905..43f016ee2 100644
--- a/src/server.c
+++ b/src/server.c
@@ -6551,7 +6551,7 @@ void loadDataFromDisk(void) {
                 }
             }
         } else if (rdb_load_ret != RDB_NOT_EXIST) {
-            serverLog(LL_WARNING,"Fatal error loading the DB: %s. Exiting.",strerror(errno));
+            serverLog(LL_WARNING, "Fatal error loading the DB, check server logs. Exiting.");
             exit(1);
         }