Add support for the PING command over the inline protocol (#412)

This PR adds, as a quick workaround to fully enable the redis-benchmark cli tool support, the ability to handle the PING command over the inline protocol. The implementation is fairly simple and works only if the PING command, followed by the new line, fits, in its entirety, the buffer otherwise is reported as error and the connection is closed. As the only reason of this implementation is to support redis-benchmark and it never sends commands byte by byte, we can always expect the command to be in the buffer. The PR also contains some minor valgrind and comment fixes.
danielealbano · May 21, 2023 · 104c08a · 104c08a
1 parent a2d7509
commit 104c08a
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 8 deletions.
diff --git a/src/module/redis/snapshot/module_redis_snapshot_load.c b/src/module/redis/snapshot/module_redis_snapshot_load.c
@@ -71,18 +71,18 @@ size_t module_redis_snapshot_load_read(
 
 uint64_t module_redis_snapshot_load_read_length_encoded_int(
         storage_channel_t *channel) {
-    uint8_t byte;
+    uint8_t byte = 0;
     uint64_t length = 0;
     module_redis_snapshot_load_read(channel, (char *) &byte, 1);
 
     if ((byte & 0xC0) == 0) {
         length = byte & 0x3F;
     } else if ((byte & 0xC0) == 0x40) {
-        uint8_t next_byte;
+        uint8_t next_byte = 0;
         module_redis_snapshot_load_read(channel, (char *) &next_byte, 1);
         length = ((byte & 0x3F) << 8) | next_byte;
     } else if ((byte & 0xC0) == 0x80 && (byte & 0x01) == 0) {
-        uint32_t length32;
+        uint32_t length32 = 0;
         module_redis_snapshot_load_read(channel, (char *) &length32, 4);
         length = int32_ntoh(length32);
     } else if ((byte & 0xC0) == 0x80 && (byte & 0x01) == 1) {
@@ -236,8 +236,8 @@ uint8_t module_redis_snapshot_load_read_opcode(
 
 void module_redis_snapshot_load_process_opcode_aux(
         storage_channel_t *channel) {
-    size_t key_length;
-    size_t value_length;
+    size_t key_length = 0;
+    size_t value_length = 0;
     void *key = module_redis_snapshot_load_read_string(channel, &key_length);
     void *value = module_redis_snapshot_load_read_string(channel, &value_length);
 
@@ -361,8 +361,8 @@ void module_redis_snapshot_load_process_value_string(
         storage_channel_t *channel,
         uint64_t expiry_ms) {
     bool set_failed = false;
-    size_t key_length;
-    size_t value_length;
+    size_t key_length = 0;
+    size_t value_length = 0;
     char *key, *value;
 
     key = module_redis_snapshot_load_read_string(channel, &key_length);

diff --git a/src/protocol/redis/protocol_redis_reader.c b/src/protocol/redis/protocol_redis_reader.c
@@ -58,8 +58,66 @@ int32_t protocol_redis_reader_read(
         bool is_inline = first_byte != PROTOCOL_REDIS_TYPE_ARRAY;
 
         if (unlikely(is_inline)) {
+            // The inline protocol support has still to be implemented by to support redis-benchmark there is an ad-hoc
+            // check for the PING command, as redis-benchmark ignores the protocol type and sends the PING command
+            // as inline.
+            // This check will also work only if the PING command is sent as PING\r\n altogether, if the command will
+            // be split among multiple packets it will not work.
+            if (length >= 6 && strncmp(buffer, "PING\r\n", 6) == 0) {
+                size_t command_length = 4;
+                size_t data_read_len = 6;
+
+                // Update the context status
+                context->arguments.count = 1;
+                context->state = PROTOCOL_REDIS_READER_STATE_COMMAND_PARSED;
+
+                // Update the various offsets (and pointers)
+                read_offset += data_read_len;
+                buffer += data_read_len;
+                context->arguments.current.received_length += data_read_len;
+
+                // Update the ops list
+                ops[op_index].type = PROTOCOL_REDIS_READER_OP_TYPE_COMMAND_BEGIN;
+                ops[op_index].data_read_len = 0;
+                ops[op_index].data.command.arguments_count = 1;
+                op_index++;
+
+                // Update the OPs list
+                ops[op_index].type = PROTOCOL_REDIS_READER_OP_TYPE_ARGUMENT_BEGIN;
+                ops[op_index].data_read_len = 0;
+                ops[op_index].data.argument.index = 0;
+                ops[op_index].data.argument.length = command_length;
+                op_index++;
+
+                // Update the OPs list
+                ops[op_index].type = PROTOCOL_REDIS_READER_OP_TYPE_ARGUMENT_DATA;
+                ops[op_index].data_read_len = (off_t)command_length;
+                ops[op_index].data.argument.index = 0;
+                ops[op_index].data.argument.length = command_length;
+                ops[op_index].data.argument.offset = 0;
+                ops[op_index].data.argument.data_length = command_length;
+                op_index++;
+
+                // Update the OPs list
+                ops[op_index].type = PROTOCOL_REDIS_READER_OP_TYPE_ARGUMENT_END;
+                ops[op_index].data_read_len = 0;
+                ops[op_index].data.argument.index = 0;
+                ops[op_index].data.argument.length = command_length;
+                ops[op_index].data.argument.offset = command_length;
+                op_index++;
+
+                // Update the OPs list
+                ops[op_index].type = PROTOCOL_REDIS_READER_OP_TYPE_COMMAND_END;
+                ops[op_index].data_read_len = 2;
+                ops[op_index].data.command.arguments_count = 1;
+                op_index++;
+
+                return op_index;
+            }
+
             context->error = PROTOCOL_REDIS_READER_ERROR_INLINE_PROTOCOL_NOT_SUPPORTED;
             return -1;
+
 //            context->protocol_type = PROTOCOL_REDIS_READER_PROTOCOL_TYPE_INLINE;
 //            context->state = PROTOCOL_REDIS_READER_STATE_INLINE_WAITING_ARGUMENT;
 //            context->arguments.count = 0;
@@ -321,7 +379,7 @@ int32_t protocol_redis_reader_read(
             return op_index;
         }
 
-        // Ensure that there is at least 1 charater and the \r before the found \n
+        // Ensure that there is at least 1 character and the \r before the found \n
         if (unlikely(new_line_ptr - buffer < 2 || *(new_line_ptr - 1) != '\r')) {
             context->error = PROTOCOL_REDIS_READER_ERROR_ARGS_ARRAY_INVALID_LENGTH;
             return -1;