From 32c5722509b9ab02de5cac41fe91373a4d624a5f Mon Sep 17 00:00:00 2001
From: huitema <huitema@huitema.net>
Date: Mon, 26 Nov 2018 21:10:05 -0800
Subject: [PATCH] Streamline stream state flags
---
picoquic/frames.c | 66 +++++++++++++++++++-----------------
picoquic/picoquic_internal.h | 57 +++++++++++++++----------------
picoquic/quicctx.c | 4 +--
picoquic/sender.c | 19 ++++++-----
4 files changed, 73 insertions(+), 73 deletions(-)
diff --git a/picoquic/frames.c b/picoquic/frames.c
index a53cc8d9..3a9be3ac 100644
--- a/picoquic/frames.c
+++ b/picoquic/frames.c
@@ -261,7 +261,8 @@ picoquic_stream_head* picoquic_find_or_create_stream(picoquic_cnx_t* cnx, uint64
} else if (!IS_BIDIR_STREAM_ID(stream_id)) {
/* Mark the stream as already finished in our direction */
- stream->stream_flags |= picoquic_stream_flag_fin_notified | picoquic_stream_flag_fin_sent;
+ stream->fin_requested = 1;
+ stream->fin_sent = 1;
}
}
@@ -311,7 +312,7 @@ int picoquic_prepare_stream_reset_frame(picoquic_cnx_t* cnx, picoquic_stream_hea
int ret = 0;
size_t byte_index = 0;
- if ((stream->stream_flags & picoquic_stream_flag_reset_requested) == 0 || (stream->stream_flags & picoquic_stream_flag_reset_sent) != 0) {
+ if (!stream->reset_requested || stream->reset_sent) {
*consumed = 0;
} else {
size_t l1 = 0, l2 = 0;
@@ -332,7 +333,8 @@ int picoquic_prepare_stream_reset_frame(picoquic_cnx_t* cnx, picoquic_stream_hea
*consumed = 0;
} else {
*consumed = byte_index;
- stream->stream_flags |= picoquic_stream_flag_reset_sent | picoquic_stream_flag_fin_sent;
+ stream->reset_sent = 1;
+ stream->fin_sent = 1;
picoquic_update_max_stream_ID_local(cnx, stream);
@@ -368,7 +370,7 @@ uint8_t* picoquic_decode_stream_reset_frame(picoquic_cnx_t* cnx, uint8_t* bytes,
} else if ((stream = picoquic_find_or_create_stream(cnx, stream_id, 1)) == NULL) {
bytes = NULL; // error already signaled
- } else if ((stream->stream_flags & (picoquic_stream_flag_fin_received | picoquic_stream_flag_reset_received)) != 0 && final_offset != stream->fin_offset) {
+ } else if ((stream->fin_received || stream->reset_received) && final_offset != stream->fin_offset) {
picoquic_connection_error(cnx, PICOQUIC_TRANSPORT_FINAL_OFFSET_ERROR,
picoquic_frame_type_reset_stream);
bytes = NULL;
@@ -376,15 +378,15 @@ uint8_t* picoquic_decode_stream_reset_frame(picoquic_cnx_t* cnx, uint8_t* bytes,
} else if (picoquic_flow_control_check_stream_offset(cnx, stream, final_offset) != 0) {
bytes = NULL; // error already signaled
- } else if ((stream->stream_flags & picoquic_stream_flag_reset_received) == 0) {
- stream->stream_flags |= picoquic_stream_flag_reset_received;
+ } else if (!stream->reset_received) {
+ stream->reset_received = 1;
stream->remote_error = error_code;
picoquic_update_max_stream_ID_local(cnx, stream);
- if (cnx->callback_fn != NULL && (stream->stream_flags & picoquic_stream_flag_reset_signalled) == 0) {
+ if (cnx->callback_fn != NULL && !stream->reset_signalled) {
cnx->callback_fn(cnx, stream->stream_id, NULL, 0, picoquic_callback_stream_reset, cnx->callback_ctx);
- stream->stream_flags |= picoquic_stream_flag_reset_signalled;
+ stream->reset_signalled = 1;
}
}
@@ -613,7 +615,8 @@ int picoquic_prepare_stop_sending_frame(picoquic_stream_head* stream,
if (bytes_max < min_length) {
ret = PICOQUIC_ERROR_FRAME_BUFFER_TOO_SMALL;
- } else if ((stream->stream_flags & picoquic_stream_flag_stop_sending_requested) == 0 || (stream->stream_flags & picoquic_stream_flag_stop_sending_sent) != 0 || (stream->stream_flags & picoquic_stream_flag_fin_received) != 0 || (stream->stream_flags & picoquic_stream_flag_reset_received) != 0) {
+ } else if (!stream->stop_sending_requested || stream->stop_sending_sent || stream->fin_received || stream->reset_received) {
+ /* no need to send a stop sending frame */
*consumed = 0;
} else {
bytes[byte_index++] = picoquic_frame_type_stop_sending;
@@ -622,7 +625,7 @@ int picoquic_prepare_stop_sending_frame(picoquic_stream_head* stream,
picoformat_16(bytes + byte_index, (uint16_t)stream->local_stop_error);
byte_index += 2;
*consumed = byte_index;
- stream->stream_flags |= picoquic_stream_flag_stop_sending_sent;
+ stream->stop_sending_sent = 1;
}
return ret;
@@ -643,13 +646,13 @@ uint8_t* picoquic_decode_stop_sending_frame(picoquic_cnx_t* cnx, uint8_t* bytes,
} else if ((stream = picoquic_find_or_create_stream(cnx, stream_id, 1)) == NULL) {
bytes = NULL; // Error already signaled
- } else if ((stream->stream_flags & (picoquic_stream_flag_stop_sending_received | picoquic_stream_flag_reset_requested)) == 0) {
- stream->stream_flags |= picoquic_stream_flag_stop_sending_received;
+ } else if (!stream->stop_sending_received && !stream->reset_requested) {
+ stream->stop_sending_received = 1;
stream->remote_stop_error = error_code;
- if (cnx->callback_fn != NULL && (stream->stream_flags & picoquic_stream_flag_stop_sending_signalled) == 0) {
+ if (cnx->callback_fn != NULL && !stream->stop_sending_signalled) {
cnx->callback_fn(cnx, stream->stream_id, NULL, 0, picoquic_callback_stop_sending, cnx->callback_ctx);
- stream->stream_flags |= picoquic_stream_flag_stop_sending_signalled;
+ stream->stop_sending_signalled = 1;
}
}
@@ -747,9 +750,9 @@ void picoquic_stream_data_callback(picoquic_cnx_t* cnx, picoquic_stream_head* st
stream->consumed_offset += data_length;
- if (stream->consumed_offset >= stream->fin_offset && (stream->stream_flags & (picoquic_stream_flag_fin_received | picoquic_stream_flag_fin_signalled)) == picoquic_stream_flag_fin_received) {
+ if (stream->consumed_offset >= stream->fin_offset && stream->fin_received && !stream->fin_signalled){
fin_now = picoquic_callback_stream_fin;
- stream->stream_flags |= picoquic_stream_flag_fin_signalled;
+ stream->fin_signalled = 1;
}
cnx->callback_fn(cnx, stream->stream_id, data->bytes + start, data_length, fin_now,
@@ -763,8 +766,8 @@ void picoquic_stream_data_callback(picoquic_cnx_t* cnx, picoquic_stream_head* st
/* handle the case where the fin frame does not carry any data */
- if (stream->consumed_offset >= stream->fin_offset && (stream->stream_flags & (picoquic_stream_flag_fin_received | picoquic_stream_flag_fin_signalled)) == picoquic_stream_flag_fin_received) {
- stream->stream_flags |= picoquic_stream_flag_fin_signalled;
+ if (stream->consumed_offset >= stream->fin_offset && stream->fin_received && !stream->fin_signalled) {
+ stream->fin_signalled = 1;
cnx->callback_fn(cnx, stream->stream_id, NULL, 0, picoquic_callback_stream_fin,
cnx->callback_ctx);
}
@@ -846,7 +849,7 @@ static int picoquic_stream_network_input(picoquic_cnx_t* cnx, uint64_t stream_id
if ((stream = picoquic_find_or_create_stream(cnx, stream_id, 1)) == NULL) {
ret = 1; // Error already signaled
- } else if ((stream->stream_flags & picoquic_stream_flag_fin_received) != 0) {
+ } else if (stream->fin_received) {
if (fin != 0 ? stream->fin_offset != new_fin_offset : new_fin_offset > stream->fin_offset) {
ret = picoquic_connection_error(cnx, PICOQUIC_TRANSPORT_FINAL_OFFSET_ERROR, 0);
@@ -854,7 +857,7 @@ static int picoquic_stream_network_input(picoquic_cnx_t* cnx, uint64_t stream_id
} else {
if (fin) {
- stream->stream_flags |= picoquic_stream_flag_fin_received;
+ stream->fin_received = 1;
should_notify = 1;
cnx->latest_progress_time = current_time;
picoquic_update_max_stream_ID_local(cnx, stream);
@@ -954,7 +957,7 @@ int picoquic_prepare_stream_frame(picoquic_cnx_t* cnx, picoquic_stream_head* str
}
if ((stream->send_queue == NULL || stream->send_queue->length <= stream->send_queue->offset) &&
- (!STREAM_FIN_NOTIFIED(stream) || STREAM_FIN_SENT(stream))) {
+ (!STREAM_FIN_REQUESTED(stream) || STREAM_FIN_SENT(stream))) {
*consumed = 0;
} else {
size_t byte_index = 0;
@@ -1040,9 +1043,9 @@ int picoquic_prepare_stream_frame(picoquic_cnx_t* cnx, picoquic_stream_head* str
}
*consumed = byte_index;
- if (ret == 0 && STREAM_FIN_NOTIFIED(stream) && stream->send_queue == 0) {
+ if (ret == 0 && STREAM_FIN_REQUESTED(stream) && stream->send_queue == 0) {
/* Set the fin bit */
- stream->stream_flags |= picoquic_stream_flag_fin_sent;
+ stream->fin_sent = 1;
bytes[0] |= 1;
picoquic_update_max_stream_ID_local(cnx, stream);
@@ -1113,7 +1116,7 @@ int picoquic_prepare_crypto_hs_frame(picoquic_cnx_t* cnx, int epoch,
int ret = 0;
picoquic_stream_head* stream = &cnx->tls_stream[epoch];
- if ((stream->send_queue == NULL || stream->send_queue->length <= stream->send_queue->offset) && ((stream->stream_flags & picoquic_stream_flag_fin_notified) == 0 || (stream->stream_flags & picoquic_stream_flag_fin_sent) != 0)) {
+ if (stream->send_queue == NULL || stream->send_queue->length <= stream->send_queue->offset) {
*consumed = 0;
} else {
size_t byte_index = 0;
@@ -1547,7 +1550,7 @@ int picoquic_check_stream_frame_already_acked(picoquic_cnx_t* cnx, uint8_t* byte
/* this is weird -- the stream was destroyed. */
*no_need_to_repeat = 1;
} else {
- if ((stream->stream_flags & picoquic_stream_flag_reset_sent) != 0) {
+ if (stream->reset_sent) {
*no_need_to_repeat = 1;
} else {
/* Check whether the ack was already received */
@@ -2163,7 +2166,7 @@ int picoquic_prepare_required_max_stream_data_frames(picoquic_cnx_t* cnx,
picoquic_stream_head* stream = cnx->first_stream;
while (stream != NULL && ret == 0 && byte_index < bytes_max) {
- if ((stream->stream_flags & (picoquic_stream_flag_fin_received | picoquic_stream_flag_reset_received)) == 0 && 2 * stream->consumed_offset > stream->maxdata_local) {
+ if (!stream->fin_received && !stream->reset_received && 2 * stream->consumed_offset > stream->maxdata_local) {
size_t bytes_in_frame = 0;
ret = picoquic_prepare_max_stream_data_frame(stream,
@@ -2233,21 +2236,20 @@ int picoquic_prepare_max_stream_ID_frame_if_needed(picoquic_cnx_t* cnx,
void picoquic_update_max_stream_ID_local(picoquic_cnx_t* cnx, picoquic_stream_head* stream)
{
- if (cnx->client_mode != IS_CLIENT_STREAM_ID(stream->stream_id) &&
- (stream->stream_flags&picoquic_stream_flag_max_stream_updated) == 0) {
+ if (cnx->client_mode != IS_CLIENT_STREAM_ID(stream->stream_id) && !stream->max_stream_updated) {
/* This is a remotely initiated stream */
- if (stream->consumed_offset >= stream->fin_offset && ((stream->stream_flags & (picoquic_stream_flag_fin_received | picoquic_stream_flag_reset_received)) != 0)) {
+ if (stream->consumed_offset >= stream->fin_offset && (stream->fin_received || stream->reset_received)) {
/* Receive is complete */
if (IS_BIDIR_STREAM_ID(stream->stream_id)) {
- if ((stream->stream_flags & (picoquic_stream_flag_fin_sent | picoquic_stream_flag_reset_sent)) != 0)
+ if (stream->fin_sent || stream->reset_sent)
{
/* Sending is complete */
- stream->stream_flags |= picoquic_stream_flag_max_stream_updated;
+ stream->max_stream_updated = 1;
cnx->max_stream_id_bidir_local_computed += 4;
}
} else {
/* No need to check receive complete on uni directional streams */
- stream->stream_flags |= picoquic_stream_flag_max_stream_updated;
+ stream->max_stream_updated = 1;
cnx->max_stream_id_unidir_local_computed += 4;
}
}
diff --git a/picoquic/picoquic_internal.h b/picoquic/picoquic_internal.h
index fcdbd60c..3d4c0ea8 100644
--- a/picoquic/picoquic_internal.h
+++ b/picoquic/picoquic_internal.h
@@ -359,11 +359,11 @@ typedef struct st_picoquic_sack_item_t {
} picoquic_sack_item_t;
/*
- * Stream head.
- * Stream contains bytes of data, which are not always delivered in order.
- * When in order data is available, the application can read it,
- * or a callback can be set.
- */
+ * Stream head.
+ * Stream contains bytes of data, which are not always delivered in order.
+ * When in order data is available, the application can read it,
+ * or a callback can be set.
+ */
typedef struct _picoquic_stream_data {
struct _picoquic_stream_data* next_stream_data;
@@ -372,22 +372,6 @@ typedef struct _picoquic_stream_data {
uint8_t* bytes;
} picoquic_stream_data;
-typedef enum picoquic_stream_flags {
- picoquic_stream_flag_fin_received = 1,
- picoquic_stream_flag_fin_signalled = 2,
- picoquic_stream_flag_fin_notified = 4,
- picoquic_stream_flag_fin_sent = 8,
- picoquic_stream_flag_reset_requested = 16,
- picoquic_stream_flag_reset_sent = 32,
- picoquic_stream_flag_reset_received = 64,
- picoquic_stream_flag_reset_signalled = 128,
- picoquic_stream_flag_stop_sending_requested = 256,
- picoquic_stream_flag_stop_sending_sent = 512,
- picoquic_stream_flag_stop_sending_received = 1024,
- picoquic_stream_flag_stop_sending_signalled = 2048,
- picoquic_stream_flag_max_stream_updated = 4096
-} picoquic_stream_flags;
-
typedef struct _picoquic_stream_head {
struct _picoquic_stream_head* next_stream;
uint64_t stream_id;
@@ -395,7 +379,6 @@ typedef struct _picoquic_stream_head {
uint64_t fin_offset;
uint64_t maxdata_local;
uint64_t maxdata_remote;
- uint32_t stream_flags;
uint32_t local_error;
uint32_t remote_error;
uint32_t local_stop_error;
@@ -404,6 +387,20 @@ typedef struct _picoquic_stream_head {
uint64_t sent_offset;
picoquic_stream_data* send_queue;
picoquic_sack_item_t first_sack_item;
+ /* Flags describing the state of the stream */
+ unsigned int fin_requested : 1; /* Application has requested Fin of sending stream */
+ unsigned int fin_sent : 1; /* Fin sent to peer */
+ unsigned int fin_received : 1; /* Fin received from peer */
+ unsigned int fin_signalled : 1; /* After Fin was received from peer, Fin was signalled to the application */
+ unsigned int reset_requested : 1; /* Application has requested to reset the stream */
+ unsigned int reset_sent : 1; /* Reset stream sent to peer */
+ unsigned int reset_received : 1; /* Reset stream received from peer */
+ unsigned int reset_signalled : 1; /* After Reset stream received from peer, application was notified */
+ unsigned int stop_sending_requested : 1; /* Application has requested to stop sending */
+ unsigned int stop_sending_sent : 1; /* Stop sending was sent to peer */
+ unsigned int stop_sending_received : 1; /* Stop sending received from peer */
+ unsigned int stop_sending_signalled : 1; /* After stop sending received from peer, application was notified */
+ unsigned int max_stream_updated : 1; /* After stream was closed in both directions, the max stream id number was updated */
} picoquic_stream_head;
#define IS_CLIENT_STREAM_ID(id) (unsigned int)(((id) & 1) == 0)
@@ -1014,16 +1011,16 @@ int picoquic_receive_transport_extensions(picoquic_cnx_t* cnx, int extension_mod
picoquic_misc_frame_header_t* picoquic_create_misc_frame(const uint8_t* bytes, size_t length);
-#define STREAM_RESET_SENT(stream) ((stream->stream_flags & picoquic_stream_flag_reset_sent) != 0)
-#define STREAM_RESET_REQUESTED(stream) ((stream->stream_flags & picoquic_stream_flag_reset_requested) != 0)
+#define STREAM_RESET_SENT(stream) (stream->reset_sent)
+#define STREAM_RESET_REQUESTED(stream) (stream->reset_requested)
#define STREAM_SEND_RESET(stream) (STREAM_RESET_REQUESTED(stream) && !STREAM_RESET_SENT(stream))
-#define STREAM_STOP_SENDING_REQUESTED(stream) ((stream->stream_flags & picoquic_stream_flag_stop_sending_requested) != 0)
-#define STREAM_STOP_SENDING_SENT(stream) ((stream->stream_flags & picoquic_stream_flag_stop_sending_sent) != 0)
-#define STREAM_STOP_SENDING_RECEIVED(stream) ((stream->stream_flags & picoquic_stream_flag_stop_sending_received) != 0)
+#define STREAM_STOP_SENDING_REQUESTED(stream) (stream->stop_sending_requested)
+#define STREAM_STOP_SENDING_SENT(stream) (stream->stop_sending_sent)
+#define STREAM_STOP_SENDING_RECEIVED(stream) (stream->stop_sending_received)
#define STREAM_SEND_STOP_SENDING(stream) (STREAM_STOP_SENDING_REQUESTED(stream) && !STREAM_STOP_SENDING_SENT(stream))
-#define STREAM_FIN_NOTIFIED(stream) ((stream->stream_flags & picoquic_stream_flag_fin_notified) != 0)
-#define STREAM_FIN_SENT(stream) ((stream->stream_flags & picoquic_stream_flag_fin_sent) != 0)
-#define STREAM_SEND_FIN(stream) (STREAM_FIN_NOTIFIED(stream) && !STREAM_FIN_SENT(stream))
+#define STREAM_FIN_REQUESTED(stream) (stream->fin_requested)
+#define STREAM_FIN_SENT(stream) (stream->fin_sent)
+#define STREAM_SEND_FIN(stream) (STREAM_FIN_REQUESTED(stream) && !STREAM_FIN_SENT(stream))
#ifdef __cplusplus
}
diff --git a/picoquic/quicctx.c b/picoquic/quicctx.c
index c0b13707..e1168ed5 100644
--- a/picoquic/quicctx.c
+++ b/picoquic/quicctx.c
@@ -1408,7 +1408,6 @@ picoquic_cnx_t* picoquic_create_cnx(picoquic_quic_t* quic,
for (int epoch = 0; epoch < PICOQUIC_NUMBER_OF_EPOCHS; epoch++) {
cnx->tls_stream[epoch].stream_id = 0;
cnx->tls_stream[epoch].consumed_offset = 0;
- cnx->tls_stream[epoch].stream_flags = 0;
cnx->tls_stream[epoch].fin_offset = 0;
cnx->tls_stream[epoch].next_stream = NULL;
cnx->tls_stream[epoch].stream_data = NULL;
@@ -1417,6 +1416,7 @@ picoquic_cnx_t* picoquic_create_cnx(picoquic_quic_t* quic,
cnx->tls_stream[epoch].remote_error = 0;
cnx->tls_stream[epoch].maxdata_local = (uint64_t)((int64_t)-1);
cnx->tls_stream[epoch].maxdata_remote = (uint64_t)((int64_t)-1);
+ /* No need to reset the state flags, as they are not used for the crypto stream */
}
cnx->congestion_alg = cnx->quic->default_congestion_alg;
@@ -1746,9 +1746,9 @@ int picoquic_reset_cnx(picoquic_cnx_t* cnx, uint64_t current_time)
for (int epoch = 0; epoch < PICOQUIC_NUMBER_OF_EPOCHS; epoch++) {
picoquic_clear_stream(&cnx->tls_stream[epoch]);
cnx->tls_stream[epoch].consumed_offset = 0;
- cnx->tls_stream[epoch].stream_flags = 0;
cnx->tls_stream[epoch].fin_offset = 0;
cnx->tls_stream[epoch].sent_offset = 0;
+ /* No need to reset the state flags, are they are not used for the crypto stream */
}
/* Reset the ECN data */
diff --git a/picoquic/sender.c b/picoquic/sender.c
index e882b3dd..a126bbb3 100644
--- a/picoquic/sender.c
+++ b/picoquic/sender.c
@@ -68,19 +68,20 @@ int picoquic_add_to_stream(picoquic_cnx_t* cnx, uint64_t stream_id,
ret = PICOQUIC_ERROR_MEMORY;
} else if (is_unidir) {
/* Mark the stream as already finished in remote direction */
- stream->stream_flags |= picoquic_stream_flag_fin_signalled | picoquic_stream_flag_fin_received;
+ stream->fin_signalled = 1;
+ stream->fin_received = 1;
}
}
}
if (ret == 0 && set_fin) {
- if ((stream->stream_flags & picoquic_stream_flag_fin_notified) != 0) {
+ if (stream->fin_requested) {
/* app error, notified the fin twice*/
if (length > 0) {
ret = -1;
}
} else {
- stream->stream_flags |= picoquic_stream_flag_fin_notified;
+ stream->fin_requested = 1;
}
}
@@ -136,12 +137,12 @@ int picoquic_reset_stream(picoquic_cnx_t* cnx,
if (stream == NULL) {
ret = PICOQUIC_ERROR_INVALID_STREAM_ID;
}
- else if ((stream->stream_flags & picoquic_stream_flag_fin_sent) != 0) {
+ else if (stream->fin_sent) {
ret = PICOQUIC_ERROR_STREAM_ALREADY_CLOSED;
}
- else if ((stream->stream_flags & picoquic_stream_flag_reset_requested) == 0) {
+ else if (!stream->reset_requested) {
stream->local_error = local_stream_error;
- stream->stream_flags |= picoquic_stream_flag_reset_requested;
+ stream->reset_requested = 1;
}
picoquic_reinsert_by_wake_time(cnx->quic, cnx, picoquic_get_quic_time(cnx->quic));
@@ -160,12 +161,12 @@ int picoquic_stop_sending(picoquic_cnx_t* cnx,
if (stream == NULL) {
ret = PICOQUIC_ERROR_INVALID_STREAM_ID;
}
- else if ((stream->stream_flags & picoquic_stream_flag_reset_received) != 0) {
+ else if (stream->reset_received) {
ret = PICOQUIC_ERROR_STREAM_ALREADY_CLOSED;
}
- else if ((stream->stream_flags & picoquic_stream_flag_stop_sending_requested) == 0) {
+ else if (!stream->stop_sending_requested) {
stream->local_stop_error = local_stream_error;
- stream->stream_flags |= picoquic_stream_flag_stop_sending_requested;
+ stream->stop_sending_requested = 1;
}
picoquic_reinsert_by_wake_time(cnx->quic, cnx, picoquic_get_quic_time(cnx->quic));
--
GitLab