MySQLnd: Simplify management of zval row buffer

Something odd was being done here, with the row packet having a
flag for whether it should allocate the zval buffer, which would
then get moved into the result set.

Keep the management of this buffer purely at the result set level.
This also allows us to easily reuse the same buffer for all results,
rather than allocating a new one for each fetch.

4 files changed, 46 insertions, 105 deletions

diff --git a/ext/mysqlnd/mysqlnd_ps.c b/ext/mysqlnd/mysqlnd_ps.c
index c7b92ab042..ba6085fce4 100644
--- a/ext/mysqlnd/mysqlnd_ps.c
+++ b/ext/mysqlnd/mysqlnd_ps.c
@@ -841,9 +841,6 @@ mysqlnd_stmt_fetch_row_unbuffered(MYSQLND_RES * result, void * param, const unsi
 		DBG_RETURN(FAIL);
 	}
 
-	/* Let the row packet fill our buffer and skip additional malloc + memcpy */
-	row_packet->skip_extraction = stmt && stmt->result_bind? FALSE:TRUE;
-
 	checkpoint = result->memory_pool->checkpoint;
 	mysqlnd_mempool_save_state(result->memory_pool);
 
@@ -854,12 +851,10 @@ mysqlnd_stmt_fetch_row_unbuffered(MYSQLND_RES * result, void * param, const unsi
 	if (PASS == (ret = PACKET_READ(conn, row_packet)) && !row_packet->eof) {
 		unsigned int i, field_count = result->field_count;
 
-		if (!row_packet->skip_extraction) {
+		if (stmt && stmt->result_bind) {
 			result->unbuf->m.free_last_data(result->unbuf, conn->stats);
 
-			result->unbuf->last_row_data = row_packet->fields;
 			result->unbuf->last_row_buffer = row_packet->row_buffer;
-			row_packet->fields = NULL;
 			row_packet->row_buffer.ptr = NULL;
 
 			if (PASS != result->unbuf->m.row_decoder(&result->unbuf->last_row_buffer,
@@ -1029,19 +1024,15 @@ mysqlnd_fetch_stmt_row_cursor(MYSQLND_RES * result, void * param, const unsigned
 
 	}
 
-	row_packet->skip_extraction = stmt->result_bind? FALSE:TRUE;
-
 	UPSERT_STATUS_RESET(stmt->upsert_status);
 	if (PASS == (ret = PACKET_READ(conn, row_packet)) && !row_packet->eof) {
 		const MYSQLND_RES_METADATA * const meta = result->meta;
 		unsigned int i, field_count = result->field_count;
 
-		if (!row_packet->skip_extraction) {
+		if (stmt->result_bind) {
 			result->unbuf->m.free_last_data(result->unbuf, conn->stats);
 
-			result->unbuf->last_row_data = row_packet->fields;
 			result->unbuf->last_row_buffer = row_packet->row_buffer;
-			row_packet->fields = NULL;
 			row_packet->row_buffer.ptr = NULL;
 
 			if (PASS != result->unbuf->m.row_decoder(&result->unbuf->last_row_buffer,
diff --git a/ext/mysqlnd/mysqlnd_result.c b/ext/mysqlnd/mysqlnd_result.c
index f67f8bdcc1..af042030ce 100644
--- a/ext/mysqlnd/mysqlnd_result.c
+++ b/ext/mysqlnd/mysqlnd_result.c
@@ -147,18 +147,12 @@ MYSQLND_METHOD(mysqlnd_result_unbuffered, free_last_data)(MYSQLND_RES_UNBUFFERED
 	}
 
 	DBG_INF_FMT("field_count=%u", unbuf->field_count);
-	if (unbuf->last_row_data) {
-		unsigned int i;
-		for (i = 0; i < unbuf->field_count; i++) {
-			zval_ptr_dtor_nogc(&(unbuf->last_row_data[i]));
-		}
-
-		/* Free last row's zvals */
-		mnd_efree(unbuf->last_row_data);
-		unbuf->last_row_data = NULL;
-	}
 	if (unbuf->last_row_buffer.ptr) {
 		DBG_INF("Freeing last row buffer");
+		for (unsigned i = 0; i < unbuf->field_count; i++) {
+			zval_ptr_dtor_nogc(&unbuf->last_row_data[i]);
+		}
+
 		/* Nothing points to this buffer now, free it */
 		unbuf->result_set_memory_pool->free_chunk(
 			unbuf->result_set_memory_pool, unbuf->last_row_buffer.ptr);
@@ -612,7 +606,7 @@ static const size_t *
 MYSQLND_METHOD(mysqlnd_result_unbuffered, fetch_lengths)(const MYSQLND_RES_UNBUFFERED * const result)
 {
 	/* simulate output of libmysql */
-	return (result->last_row_data || result->eof_reached)? result->lengths : NULL;
+	return (result->last_row_buffer.ptr || result->eof_reached)? result->lengths : NULL;
 }
 /* }}} */
 
@@ -660,8 +654,6 @@ MYSQLND_METHOD(mysqlnd_result_unbuffered, fetch_row_c)(MYSQLND_RES * result, voi
 		/* Not fully initialized object that is being cleaned up */
 		DBG_RETURN(FAIL);
 	}
-	/* Let the row packet fill our buffer and skip additional mnd_malloc + memcpy */
-	row_packet->skip_extraction = FALSE;
 
 	checkpoint = result->memory_pool->checkpoint;
 	mysqlnd_mempool_save_state(result->memory_pool);
@@ -673,52 +665,48 @@ MYSQLND_METHOD(mysqlnd_result_unbuffered, fetch_row_c)(MYSQLND_RES * result, voi
 	if (PASS == (ret = PACKET_READ(conn, row_packet)) && !row_packet->eof) {
 		result->unbuf->m.free_last_data(result->unbuf, conn->stats);
 
-		result->unbuf->last_row_data = row_packet->fields;
 		result->unbuf->last_row_buffer = row_packet->row_buffer;
-		row_packet->fields = NULL;
 		row_packet->row_buffer.ptr = NULL;
 
 		MYSQLND_INC_CONN_STATISTIC(conn->stats, STAT_ROWS_FETCHED_FROM_CLIENT_NORMAL_UNBUF);
 
-		if (!row_packet->skip_extraction) {
-			unsigned int i, field_count = meta->field_count;
+		unsigned int i, field_count = meta->field_count;
+
+		enum_func_status rc = result->unbuf->m.row_decoder(&result->unbuf->last_row_buffer,
+										result->unbuf->last_row_data,
+										field_count,
+										row_packet->fields_metadata,
+										conn->options->int_and_float_native,
+										conn->stats);
+		if (PASS != rc) {
+			mysqlnd_mempool_restore_state(result->memory_pool);
+			result->memory_pool->checkpoint = checkpoint;
+			DBG_RETURN(FAIL);
+		}
+		{
+			*row = mnd_emalloc(field_count * sizeof(char *));
+			MYSQLND_FIELD * field = meta->fields;
+			size_t * lengths = result->unbuf->lengths;
 
-			enum_func_status rc = result->unbuf->m.row_decoder(&result->unbuf->last_row_buffer,
-											result->unbuf->last_row_data,
-											field_count,
-											row_packet->fields_metadata,
-											conn->options->int_and_float_native,
-											conn->stats);
-			if (PASS != rc) {
-				mysqlnd_mempool_restore_state(result->memory_pool);
-				result->memory_pool->checkpoint = checkpoint;
-				DBG_RETURN(FAIL);
-			}
-			{
-				*row = mnd_emalloc(field_count * sizeof(char *));
-				MYSQLND_FIELD * field = meta->fields;
-				size_t * lengths = result->unbuf->lengths;
-
-				for (i = 0; i < field_count; i++, field++) {
-					zval * data = &result->unbuf->last_row_data[i];
-					const size_t len = (Z_TYPE_P(data) == IS_STRING)? Z_STRLEN_P(data) : 0;
+			for (i = 0; i < field_count; i++, field++) {
+				zval * data = &result->unbuf->last_row_data[i];
+				const size_t len = (Z_TYPE_P(data) == IS_STRING)? Z_STRLEN_P(data) : 0;
 
 /* BEGIN difference between normal normal fetch and _c */
-					if (Z_TYPE_P(data) != IS_NULL) {
-						convert_to_string(data);
-						(*row)[i] = Z_STRVAL_P(data);
-					} else {
-						(*row)[i] = NULL;
-					}
+				if (Z_TYPE_P(data) != IS_NULL) {
+					convert_to_string(data);
+					(*row)[i] = Z_STRVAL_P(data);
+				} else {
+					(*row)[i] = NULL;
+				}
 /* END difference between normal normal fetch and _c */
 
-					if (lengths) {
-						lengths[i] = len;
-					}
+				if (lengths) {
+					lengths[i] = len;
+				}
 
-					if (field->max_length < len) {
-						field->max_length = len;
-					}
+				if (field->max_length < len) {
+					field->max_length = len;
 				}
 			}
 		}
@@ -788,8 +776,6 @@ MYSQLND_METHOD(mysqlnd_result_unbuffered, fetch_row)(MYSQLND_RES * result, void
 		/* Not fully initialized object that is being cleaned up */
 		DBG_RETURN(FAIL);
 	}
-	/* Let the row packet fill our buffer and skip additional mnd_malloc + memcpy */
-	row_packet->skip_extraction = row? FALSE:TRUE;
 
 	checkpoint = result->memory_pool->checkpoint;
 	mysqlnd_mempool_save_state(result->memory_pool);
@@ -801,14 +787,12 @@ MYSQLND_METHOD(mysqlnd_result_unbuffered, fetch_row)(MYSQLND_RES * result, void
 	if (PASS == (ret = PACKET_READ(conn, row_packet)) && !row_packet->eof) {
 		result->unbuf->m.free_last_data(result->unbuf, conn->stats);
 
-		result->unbuf->last_row_data = row_packet->fields;
 		result->unbuf->last_row_buffer = row_packet->row_buffer;
-		row_packet->fields = NULL;
 		row_packet->row_buffer.ptr = NULL;
 
 		MYSQLND_INC_CONN_STATISTIC(conn->stats, STAT_ROWS_FETCHED_FROM_CLIENT_NORMAL_UNBUF);
 
-		if (!row_packet->skip_extraction) {
+		if (row) {
 			unsigned int i, field_count = meta->field_count;
 
 			enum_func_status rc = result->unbuf->m.row_decoder(&result->unbuf->last_row_buffer,
@@ -1263,8 +1247,6 @@ MYSQLND_METHOD(mysqlnd_res, store_result_fetch_data)(MYSQLND_CONN_DATA * const c
 	row_packet.binary_protocol = binary_protocol;
 	row_packet.fields_metadata = meta->fields;
 
-	row_packet.skip_extraction = TRUE; /* let php_mysqlnd_rowp_read() not allocate row_packet.fields, we will do it */
-
 	while (FAIL != (ret = PACKET_READ(conn, &row_packet)) && !row_packet.eof) {
 		if (!free_rows) {
 			MYSQLND_ROW_BUFFER * new_row_buffers;
@@ -1301,7 +1283,6 @@ MYSQLND_METHOD(mysqlnd_res, store_result_fetch_data)(MYSQLND_CONN_DATA * const c
 		set->row_count++;
 
 		/* So row_packet's destructor function won't efree() it */
-		row_packet.fields = NULL;
 		row_packet.row_buffer.ptr = NULL;
 
 		/*
@@ -1459,7 +1440,9 @@ MYSQLND_METHOD(mysqlnd_res, skip_result)(MYSQLND_RES * const result)
 																		STAT_FLUSHED_PS_SETS);
 
 		while ((PASS == result->m.fetch_row(result, NULL, 0, &fetched_anything)) && fetched_anything == TRUE) {
-			/* do nothing */;
+			MYSQLND_INC_CONN_STATISTIC(conn->stats,
+				result->type == MYSQLND_RES_NORMAL
+					? STAT_ROWS_SKIPPED_NORMAL : STAT_ROWS_SKIPPED_PS);
 		}
 	}
 	DBG_RETURN(PASS);
@@ -1880,6 +1863,9 @@ mysqlnd_result_unbuffered_init(MYSQLND_RES *result, const unsigned int field_cou
 	ret->lengths = pool->get_chunk(pool, field_count * sizeof(size_t));
 	memset(ret->lengths, 0, field_count * sizeof(size_t));
 
+	ret->last_row_data = pool->get_chunk(pool, field_count * sizeof(zval));
+	memset(ret->last_row_data, 0, field_count * sizeof(zval));
+
 	ret->result_set_memory_pool = pool;
 	ret->field_count= field_count;
 	ret->ps = ps;
diff --git a/ext/mysqlnd/mysqlnd_wireprotocol.c b/ext/mysqlnd/mysqlnd_wireprotocol.c
index ef32d90274..d33e266b6d 100644
--- a/ext/mysqlnd/mysqlnd_wireprotocol.c
+++ b/ext/mysqlnd/mysqlnd_wireprotocol.c
@@ -1696,10 +1696,6 @@ php_mysqlnd_rowp_read_text_protocol_c(MYSQLND_ROW_BUFFER * row_buffer, zval * fi
 
 
 /* {{{ php_mysqlnd_rowp_read */
-/*
-  if normal statements => packet->fields is created by this function,
-  if PS => packet->fields is passed from outside
-*/
 static enum_func_status
 php_mysqlnd_rowp_read(MYSQLND_CONN_DATA * conn, void * _packet)
 {
@@ -1760,33 +1756,10 @@ php_mysqlnd_rowp_read(MYSQLND_CONN_DATA * conn, void * _packet)
 			DBG_INF_FMT("server_status=%u warning_count=%u", packet->server_status, packet->warning_count);
 		}
 	} else {
+		packet->eof = FALSE;
 		MYSQLND_INC_CONN_STATISTIC(stats,
 									packet->binary_protocol? STAT_ROWS_FETCHED_FROM_SERVER_PS:
 															 STAT_ROWS_FETCHED_FROM_SERVER_NORMAL);
-
-		packet->eof = FALSE;
-		/* packet->field_count is set by the user of the packet */
-
-		if (!packet->skip_extraction) {
-			if (!packet->fields) {
-				DBG_INF("Allocating packet->fields");
-				/*
-				  old-API will probably set packet->fields to NULL every time, though for
-				  unbuffered sets it makes not much sense as the zvals in this buffer matter,
-				  not the buffer. Constantly allocating and deallocating brings nothing.
-
-				  For PS - if stmt_store() is performed, thus we don't have a cursor, it will
-				  behave just like old-API buffered. Cursors will behave like a bit different,
-				  but mostly like old-API unbuffered and thus will populate this array with
-				  value.
-				*/
-				packet->fields = mnd_ecalloc(packet->field_count, sizeof(zval));
-			}
-		} else {
-			MYSQLND_INC_CONN_STATISTIC(stats,
-										packet->binary_protocol? STAT_ROWS_SKIPPED_PS:
-																 STAT_ROWS_SKIPPED_NORMAL);
-		}
 	}
 
 end:
@@ -1807,13 +1780,6 @@ php_mysqlnd_rowp_free_mem(void * _packet)
 		p->result_set_memory_pool->free_chunk(p->result_set_memory_pool, p->row_buffer.ptr);
 		p->row_buffer.ptr = NULL;
 	}
-	/*
-	  Don't free packet->fields :
-	  - normal queries -> store_result() | fetch_row_unbuffered() will transfer
-	    the ownership and NULL it.
-	  - PS will pass in it the bound variables, we have to use them! and of course
-	    not free the array. As it is passed to us, we should not clean it ourselves.
-	*/
 	DBG_VOID_RETURN;
 }
 /* }}} */
diff --git a/ext/mysqlnd/mysqlnd_wireprotocol.h b/ext/mysqlnd/mysqlnd_wireprotocol.h
index e917736bb5..28d521644e 100644
--- a/ext/mysqlnd/mysqlnd_wireprotocol.h
+++ b/ext/mysqlnd/mysqlnd_wireprotocol.h
@@ -206,7 +206,6 @@ typedef struct st_mysqlnd_packet_res_field {
 /* Row packet */
 typedef struct st_mysqlnd_packet_row {
 	MYSQLND_PACKET_HEADER	header;
-	zval		*fields;
 	uint32_t	field_count;
 	zend_bool	eof;
 	/*
@@ -219,7 +218,6 @@ typedef struct st_mysqlnd_packet_row {
 	MYSQLND_ROW_BUFFER	row_buffer;
 	MYSQLND_MEMORY_POOL * result_set_memory_pool;
 
-	zend_bool		skip_extraction;
 	zend_bool		binary_protocol;
 	MYSQLND_FIELD	*fields_metadata;