MXS-1931: Remove use of gw_MySQL_get_next_packet

The function implemented redundant functionality and replacement with
modutil_get_next_MySQL_packet was planned.

When faced with a packet header spread over multiple buffers, the packet
length calculation would read past the buffer end. This is fixed by taking
modutil_get_next_MySQL_packet into use.

Identical behavior to the old function is achieved by calling
gwbuf_make_contiguous for each packet to store them in a contiguous area
of memory. This should be either removed and only done when
RCAP_TYPE_CONTIGUOUS_INPUT is requested or be made an innate feature of
statement based routing.

include/maxscale/protocol/mysql.h

@@ -464,8 +464,6 @@ int mysql_send_custom_error(DCB *dcb, int sequence, int affected_rows, const cha
 int mysql_send_standard_error(DCB *dcb, int sequence, int errnum, const char *msg);
 int mysql_send_auth_error(DCB *dcb, int sequence, int affected_rows, const char* msg);
 
-GWBUF* gw_MySQL_get_next_packet(GWBUF** p_readbuf);
-GWBUF* gw_MySQL_get_packets(GWBUF** p_readbuf, int* npackets);
 void   protocol_add_srv_command(MySQLProtocol* p, mxs_mysql_cmd_t cmd);
 void   protocol_remove_srv_command(MySQLProtocol* p);
 bool   protocol_waits_response(MySQLProtocol* p);

server/modules/protocol/MySQL/mariadbclient/mysql_client.cc

@@ -1576,6 +1576,17 @@ static bool reauthenticate_client(MXS_SESSION* session, GWBUF* packetbuf)
     return rval;
 }
 
+// Helper function for debug assertions
+static bool only_one_packet(GWBUF* buffer)
+{
+    ss_dassert(buffer);
+    uint8_t header[4] = {};
+    gwbuf_copy_data(buffer, 0, MYSQL_HEADER_LEN, header);
+    size_t packet_len = gw_mysql_get_byte3(header);
+    size_t buffer_len = gwbuf_length(buffer);
+    return packet_len + MYSQL_HEADER_LEN == buffer_len;
+}
+
 /**
  * Detect if buffer includes partial mysql packet or multiple packets.
  * Store partial packet to dcb_readqueue. Send complete packets one by one
@@ -1597,15 +1608,15 @@ static int route_by_statement(MXS_SESSION* session, uint64_t capabilities, GWBUF
     GWBUF* packetbuf;
     do
     {
-        /**
+        // Process client request one packet at a time
-         * Collect incoming bytes to a buffer until complete packet has
+        packetbuf = modutil_get_next_MySQL_packet(p_readbuf);
-         * arrived and then return the buffer.
+
-         */
+        // TODO: Do this only when RCAP_TYPE_CONTIGUOUS_INPUT is requested
-        // TODO: This should be replaced with modutil_get_next_MySQL_packet.
+        packetbuf = gwbuf_make_contiguous(packetbuf);
-        packetbuf = gw_MySQL_get_next_packet(p_readbuf);
 
         if (packetbuf != NULL)
         {
+            ss_dassert(only_one_packet(packetbuf));
             CHK_GWBUF(packetbuf);
             MySQLProtocol* proto = (MySQLProtocol*)session->client_dcb->protocol;
 
@@ -1619,24 +1630,7 @@ static int route_by_statement(MXS_SESSION* session, uint64_t capabilities, GWBUF
 
             if (rcap_type_required(capabilities, RCAP_TYPE_CONTIGUOUS_INPUT))
             {
-                if (!GWBUF_IS_CONTIGUOUS(packetbuf))
+                ss_dassert(GWBUF_IS_CONTIGUOUS(packetbuf));
-                {
-                    // TODO: As long as gw_MySQL_get_next_packet is used above, the buffer
-                    // TODO: will be contiguous. That function should be replaced with
-                    // TODO: modutil_get_next_MySQL_packet.
-                    GWBUF* tmp = gwbuf_make_contiguous(packetbuf);
-                    if (tmp)
-                    {
-                        packetbuf = tmp;
-                    }
-                    else
-                    {
-                        // TODO: A memory allocation failure. We should close the dcb
-                        // TODO: and terminate the session.
-                        rc = 0;
-                        goto return_rc;
-                    }
-                }
 
                 if (rcap_type_required(capabilities, RCAP_TYPE_TRANSACTION_TRACKING))
                 {

server/modules/protocol/MySQL/mysql_common.cc

@@ -473,100 +473,6 @@ int mysql_send_auth_error(DCB        *dcb,
     return sizeof(mysql_packet_header) + mysql_payload_size;
 }
 
-
-/**
- * Buffer contains at least one of the following:
- * complete [complete] [partial] mysql packet
- *
- * @param p_readbuf     Address of read buffer pointer
- *
- * @return pointer to gwbuf containing a complete packet or
- *   NULL if no complete packet was found.
- */
-GWBUF* gw_MySQL_get_next_packet(GWBUF** p_readbuf)
-{
-    GWBUF* packetbuf;
-    GWBUF* readbuf;
-    size_t buflen;
-    size_t packetlen;
-    size_t totalbuflen;
-    uint8_t* data;
-    size_t nbytes_copied = 0;
-    uint8_t* target;
-
-    readbuf = *p_readbuf;
-
-    if (readbuf == NULL)
-    {
-        packetbuf = NULL;
-        goto return_packetbuf;
-    }
-    CHK_GWBUF(readbuf);
-
-    if (GWBUF_EMPTY(readbuf))
-    {
-        packetbuf = NULL;
-        goto return_packetbuf;
-    }
-    totalbuflen = gwbuf_length(readbuf);
-    data = (uint8_t *)GWBUF_DATA((readbuf));
-    packetlen = MYSQL_GET_PAYLOAD_LEN(data) + 4;
-
-    /** packet is incomplete */
-    if (packetlen > totalbuflen)
-    {
-        packetbuf = NULL;
-        goto return_packetbuf;
-    }
-
-    packetbuf = gwbuf_alloc(packetlen);
-    target = GWBUF_DATA(packetbuf);
-    packetbuf->gwbuf_type = readbuf->gwbuf_type; /*< Copy the type too */
-    /**
-     * Copy first MySQL packet to packetbuf and leave posible other
-     * packets to read buffer.
-     */
-    while (nbytes_copied < packetlen && totalbuflen > 0)
-    {
-        uint8_t* src = GWBUF_DATA((*p_readbuf));
-        size_t   bytestocopy;
-
-        buflen = GWBUF_LENGTH((*p_readbuf));
-        bytestocopy = buflen < (packetlen - nbytes_copied) ? buflen : packetlen - nbytes_copied;
-
-        memcpy(target + nbytes_copied, src, bytestocopy);
-        *p_readbuf = gwbuf_consume((*p_readbuf), bytestocopy);
-        totalbuflen = gwbuf_length((*p_readbuf));
-        nbytes_copied += bytestocopy;
-    }
-    ss_dassert(buflen == 0 || nbytes_copied == packetlen);
-
-return_packetbuf:
-    return packetbuf;
-}
-
-/**
- * Move <npackets> from buffer pointed to by <*p_readbuf>.
- * Appears to be unused 11 May 2016 (Martin)
- */
-GWBUF* gw_MySQL_get_packets(GWBUF** p_srcbuf,
-                            int*    npackets)
-{
-    GWBUF* packetbuf;
-    GWBUF* targetbuf = NULL;
-
-    while (*npackets > 0 && (packetbuf = gw_MySQL_get_next_packet(p_srcbuf)) != NULL)
-    {
-        targetbuf = gwbuf_append(targetbuf, packetbuf);
-        *npackets -= 1;
-    }
-    ss_dassert(*npackets < 128);
-    ss_dassert(*npackets >= 0);
-
-    return targetbuf;
-}
-
-
 static server_command_t* server_command_init(server_command_t* srvcmd,
                                              mxs_mysql_cmd_t cmd)
 {