[gnutls-devel] [PATCH] Broke apart _gnutls_recv_int() to the packet and non-packet cases.

Jaak Ristioja jaak.ristioja at cyber.ee
Mon Feb 15 11:14:52 CET 2016


Only gnutls_record_recv_packet() called _gnutls_recv_int() with
(packet != NULL). I refactored this logic directly downstream into
gnutls_record_recv_packet(). The _gnutls_recv_int() function now only
handles non-packet specific logic. The _gnutls_recv_int2() function
was created to deduplicate common code which would otherwise have
ended up in both functions.

The rationale behind this change is to optimize what were previously
calls of _gnutls_recv_int(). First of all _gnutls_recv_int() now has
only 6 parameters, which according to the x86_64 System V Application
Binary Interface should now fit into CPU registers and no longer use
the stack. Secondly this change avoids a number of branching checks
for both packet and non-packet cases.
---
 lib/ext/heartbeat.c |   2 +-
 lib/handshake.c     |   2 +-
 lib/record.c        | 103 ++++++++++++++++++++++++++++++----------------------
 lib/record.h        |   1 -
 4 files changed, 62 insertions(+), 46 deletions(-)

diff --git a/lib/ext/heartbeat.c b/lib/ext/heartbeat.c
index cc13508..77d990b 100644
--- a/lib/ext/heartbeat.c
+++ b/lib/ext/heartbeat.c
@@ -229,7 +229,7 @@ gnutls_heartbeat_ping(gnutls_session_t session, size_t data_size,
 
 	case SHB_RECV:
 		ret =
-		    _gnutls_recv_int(session, GNUTLS_HEARTBEAT, NULL,
+		    _gnutls_recv_int(session, GNUTLS_HEARTBEAT,
 				     NULL, 0, NULL,
 				     session->internals.
 				     hb_actual_retrans_timeout_ms);
diff --git a/lib/handshake.c b/lib/handshake.c
index 3d7a153..82f605b 100644
--- a/lib/handshake.c
+++ b/lib/handshake.c
@@ -3104,7 +3104,7 @@ static int recv_handshake_final(gnutls_session_t session, int init)
 
 		ret =
 		    _gnutls_recv_int(session, GNUTLS_CHANGE_CIPHER_SPEC,
-				     NULL, ccs, ccs_len, NULL, tleft);
+				     ccs, ccs_len, NULL, tleft);
 		if (ret <= 0) {
 			ERR("recv ChangeCipherSpec", ret);
 			gnutls_assert();
diff --git a/lib/record.c b/lib/record.c
index a2f8637..713dd94 100644
--- a/lib/record.c
+++ b/lib/record.c
@@ -301,7 +301,7 @@ int gnutls_bye(gnutls_session_t session, gnutls_close_request_t how)
 			do {
 				ret =
 				    _gnutls_recv_int(session, GNUTLS_ALERT,
-						     NULL, NULL, 0, NULL,
+						     NULL, 0, NULL,
 						     session->internals.
 						     record_timeout_ms);
 			}
@@ -1356,23 +1356,13 @@ _gnutls_recv_in_buffers(gnutls_session_t session, content_type_t type,
 		return ret;
 }
 
-/* This function behaves exactly like read(). The only difference is
- * that it accepts the gnutls_session_t and the content_type_t of data to
- * receive (if called by the user the Content is Userdata only)
- * It is intended to receive data, under the current session.
- */
-ssize_t
-_gnutls_recv_int(gnutls_session_t session, content_type_t type,
-		 gnutls_packet_t *packet,
-		 uint8_t * data, size_t data_size, void *seq,
-		 unsigned int ms)
+/* Returns a value greater than zero (>= 0) if buffers should be checked
+ * for data. */
+static ssize_t
+_gnutls_recv_int2(gnutls_session_t session)
 {
 	int ret;
 
-	if (packet == NULL && (type != GNUTLS_ALERT && type != GNUTLS_HEARTBEAT)
-	    && (data_size == 0 || data == NULL))
-		return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST);
-
 	if (session->internals.read_eof != 0) {
 		/* if we have already read an EOF
 		 */
@@ -1390,37 +1380,48 @@ _gnutls_recv_int(gnutls_session_t session, content_type_t type,
 			return gnutls_assert_val(ret);
 
 		session->internals.recv_state = RECV_STATE_0;
+		/* Fall through: */
 	case RECV_STATE_0:
 
 		_dtls_async_timer_check(session);
+		return 1;
+	default:
+		return gnutls_assert_val(GNUTLS_E_INTERNAL_ERROR);
+	}
+}
 
-		if (packet == NULL) {
-			/* If we have enough data in the cache do not bother receiving
-			 * a new packet. (in order to flush the cache)
-			 */
-			ret = check_buffers(session, type, data, data_size, seq);
-			if (ret != 0)
-				return ret;
+/* This function behaves exactly like read(). The only difference is
+ * that it accepts the gnutls_session_t and the content_type_t of data to
+ * receive (if called by the user the Content is Userdata only)
+ * It is intended to receive data, under the current session.
+ */
+ssize_t
+_gnutls_recv_int(gnutls_session_t session, content_type_t type,
+		 uint8_t * data, size_t data_size, void *seq,
+		 unsigned int ms)
+{
+	int ret;
 
-			ret = _gnutls_recv_in_buffers(session, type, -1, ms);
-			if (ret < 0 && ret != GNUTLS_E_SESSION_EOF)
-				return gnutls_assert_val(ret);
+	if ((type != GNUTLS_ALERT && type != GNUTLS_HEARTBEAT)
+	    && (data_size == 0 || data == NULL))
+		return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST);
 
-			return check_buffers(session, type, data, data_size, seq);
-		} else {
-			ret = check_packet_buffers(session, type, packet);
-			if (ret != 0)
-				return ret;
+	ret = _gnutls_recv_int2(session);
+	if (ret <= 0)
+		return ret;
 
-			ret = _gnutls_recv_in_buffers(session, type, -1, ms);
-			if (ret < 0 && ret != GNUTLS_E_SESSION_EOF)
-				return gnutls_assert_val(ret);
+	/* If we have enough data in the cache do not bother receiving
+	 * a new packet. (in order to flush the cache)
+	 */
+	ret = check_buffers(session, type, data, data_size, seq);
+	if (ret != 0)
+		return ret;
 
-			return check_packet_buffers(session, type, packet);
-		}
-	default:
-		return gnutls_assert_val(GNUTLS_E_INTERNAL_ERROR);
-	}
+	ret = _gnutls_recv_in_buffers(session, type, -1, ms);
+	if (ret < 0 && ret != GNUTLS_E_SESSION_EOF)
+		return gnutls_assert_val(ret);
+
+	return check_buffers(session, type, data, data_size, seq);
 }
 
 /**
@@ -1511,9 +1512,25 @@ ssize_t
 gnutls_record_recv_packet(gnutls_session_t session, 
 		   	  gnutls_packet_t *packet)
 {
-	return _gnutls_recv_int(session, GNUTLS_APPLICATION_DATA, packet,
-				NULL, 0, NULL,
-				session->internals.record_timeout_ms);
+	int ret;
+
+	if (packet == NULL)
+		return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST);
+
+	ret = _gnutls_recv_int2(session);
+	if (ret <= 0)
+		return ret;
+
+	ret = check_packet_buffers(session, GNUTLS_APPLICATION_DATA, packet);
+	if (ret != 0)
+		return ret;
+
+	ret = _gnutls_recv_in_buffers(session, GNUTLS_APPLICATION_DATA, -1,
+	                              session->internals.record_timeout_ms);
+	if (ret < 0 && ret != GNUTLS_E_SESSION_EOF)
+		return gnutls_assert_val(ret);
+
+	return check_packet_buffers(session, GNUTLS_APPLICATION_DATA, packet);
 }
 
 /**
@@ -1694,7 +1711,7 @@ int gnutls_record_uncork(gnutls_session_t session, unsigned int flags)
 ssize_t
 gnutls_record_recv(gnutls_session_t session, void *data, size_t data_size)
 {
-	return _gnutls_recv_int(session, GNUTLS_APPLICATION_DATA, NULL,
+	return _gnutls_recv_int(session, GNUTLS_APPLICATION_DATA,
 				data, data_size, NULL,
 				session->internals.record_timeout_ms);
 }
@@ -1723,7 +1740,7 @@ ssize_t
 gnutls_record_recv_seq(gnutls_session_t session, void *data,
 		       size_t data_size, unsigned char *seq)
 {
-	return _gnutls_recv_int(session, GNUTLS_APPLICATION_DATA, NULL,
+	return _gnutls_recv_int(session, GNUTLS_APPLICATION_DATA,
 				data, data_size, seq,
 				session->internals.record_timeout_ms);
 }
diff --git a/lib/record.h b/lib/record.h
index d029586..f16df47 100644
--- a/lib/record.h
+++ b/lib/record.h
@@ -45,7 +45,6 @@ _gnutls_send_int(gnutls_session_t session, content_type_t type,
 }
 
 ssize_t _gnutls_recv_int(gnutls_session_t session, content_type_t type,
-			 gnutls_packet_t *packet,
 			 uint8_t * data,
 			 size_t sizeofdata, void *seq, unsigned int ms);
 
-- 
2.7.1




More information about the Gnutls-devel mailing list