pr fixes

redis · Dec 4, 2023 · 5496636 · 5496636
1 parent f95e3fd
commit 5496636
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 14 deletions.
diff --git a/api/librdb-api.h b/api/librdb-api.h
@@ -48,6 +48,9 @@ typedef enum RdbRes {
     RDB_ERR_INVALID_CONFIGURATION,
     RDB_ERR_FAILED_CREATE_PARSER,
     RDB_ERR_FAILED_OPEN_LOG_FILE,
+    RDB_ERR_FAILED_GET_FD_FLAGS,
+    RDB_ERR_NONBLOCKING_FD,
+    RDB_ERR_NONBLOCKING_READ_FD,
     RDB_ERR_FAILED_READ_RDB_FILE,
     RDB_ERR_FAILED_OPEN_RDB_FILE,
     RDB_ERR_WRONG_FILE_SIGNATURE,
@@ -352,9 +355,13 @@ _LIBRDB_API RdbStatus RDB_parseBuff(RdbParser *p,
  * Used by:   RDBX_createReaderFile
  *           <user-defined-reader>
  *
- * TODO: Extend RdbReaderFunc to return number of bytes read actually read and
- *       adapt parser accordingly. (Currently it can either return success,
- *       fail or wait-more-data)
+ * TODO:     The parser only supports reading asynchrnously (non-blocking)
+ *           through RDB_parseBuff() API. It is required to Extend parser for
+ *           readers to support non-blocking mode as well. Currently the provided
+ *           reader-function (RdbReaderFunc) can only:
+ *           - Read the entire request and return RDB_STATUS_OK
+ *           - Or, read none and return RDB_STATUS_WAIT_MORE_DATA
+ *           - Or, return RDB_STATUS_ERROR
  ****************************************************************/
 _LIBRDB_API RdbReader *RDB_createReaderRdb(RdbParser *p,
                                            RdbReaderFunc r,

diff --git a/api/librdb-ext-api.h b/api/librdb-ext-api.h
@@ -54,6 +54,14 @@ typedef enum {
 
 /****************************************************************
  * Create RDB Reader
+ *
+ * Creation of RDB reader based on filename or file-descriptor
+ *
+ * Note: File-descriptor must be set to blocking mode.
+ *
+ * TODO: The parser only supports reading asynchronously (non-blocking)
+ *       through RDB_parseBuff() API. It is required to Extend parser for
+ *       readers to support non-blocking mode as well.
  ****************************************************************/
 
 _LIBRDB_API RdbxReaderFile *RDBX_createReaderFile(RdbParser *parser, const char *filename);

diff --git a/src/ext/readerFileDesc.c b/src/ext/readerFileDesc.c
@@ -1,5 +1,7 @@
 #include <stdio.h>
+#include <fcntl.h>
 #include <string.h>
+#include <assert.h>
 #include <errno.h>
 #include <unistd.h>
 #include "common.h"
@@ -27,19 +29,33 @@ static RdbStatus readFileDesc(void *data, void *buf, size_t len) {
     while (totalBytesRead < len) {
         ssize_t bytesRead = read(ctx->fd, (char *)buf + totalBytesRead, len - totalBytesRead);
 
-        if (bytesRead == -1) {
-            if (errno != EINTR) {
-                RDB_reportError(ctx->parser, RDB_ERR_FAILED_READ_RDB_FILE,
-                                "readFileDesc(): Read failed with errno=%d", errno);
-                return RDB_STATUS_ERROR;
-            }
+        /* read some data */
+        if (likely(bytesRead > 0)) {
+            totalBytesRead += bytesRead;
+            break;
+        }
 
-            continue; /* If interrupted, retry the read */
-        } else if (bytesRead == 0) {
+        /* didn't read any data. Stop. */
+        if (bytesRead == 0) {
             break;
-        } else {
-            totalBytesRead += bytesRead;
         }
+
+        assert(bytesRead == -1);
+
+        /* If interrupted, retry read */
+        if (errno == EINTR)
+            continue;
+
+        /* Wrongly configured to nonblocking mode (Not supported at the moment) */
+        if (errno == EAGAIN || errno == EWOULDBLOCK) {
+            RDB_reportError(ctx->parser, RDB_ERR_NONBLOCKING_READ_FD,
+                            "readFileDesc(): Unexpected EAGAIN|EWOULDBLOCK. The fd must be set to blocking mode");
+            return RDB_STATUS_ERROR;
+        }
+
+        RDB_reportError(ctx->parser, RDB_ERR_FAILED_READ_RDB_FILE,
+                        "readFileDesc(): Read failed with errno=%d", errno);
+        return RDB_STATUS_ERROR;
     }
 
     if (totalBytesRead < len) {
@@ -52,6 +68,22 @@ static RdbStatus readFileDesc(void *data, void *buf, size_t len) {
 }
 
 RdbxReaderFileDesc *RDBX_createReaderFileDesc(RdbParser *p, int fd, int fdCloseWhenDone) {
+
+    int flags = fcntl(fd, F_GETFL);
+
+    if (flags==-1) {
+
+        RDB_reportError(p, RDB_ERR_FAILED_GET_FD_FLAGS,
+            "RDBX_createReaderFileDesc(): Error getting file descriptor flags");
+        return NULL;
+    }
+
+    if (flags & O_NONBLOCK) {
+        RDB_reportError(p, RDB_ERR_NONBLOCKING_FD,
+            "RDBX_createReaderFileDesc(): fd must be set to blocking mode");
+        return NULL;
+    }
+
     RdbxReaderFileDesc *ctx = (RdbxReaderFileDesc *) RDB_alloc(p, sizeof(RdbxReaderFileDesc));
     ctx->parser = p;
     ctx->fd = fd;

diff --git a/src/lib/parserRaw.c b/src/lib/parserRaw.c
@@ -845,7 +845,7 @@ RdbStatus elementRawStreamLP(RdbParser *p) {
                     return RDB_STATUS_ERROR;
                 }
             }
-            return updateElementState(p, ST_LOAD_NEXT_LP_IS_MORE, 1); /* fall-thru */
+            return updateElementState(p, ST_LOAD_NEXT_LP_IS_MORE, 1);
 
         case ST_LOAD_METADATA: {
                 uint64_t dummyVal;