BUG/MINOR: quic: Wrong returned status by qc_build_frms()
This function must return a successful status as soon as it could be build
a frame to be embedded by a packet. This behavior was broken by the last
modifications. This was due to a dangerous "ret = 1" statement inside
a loop. This statement must be reach only if we go out of a switch/case
after a "break" statement.
Add comments to mention this information.
diff --git a/src/xprt_quic.c b/src/xprt_quic.c
index e11b8b6..9eeef95 100644
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -5440,7 +5440,7 @@
*
* Update consequently <*len> to reflect the size of these frames built
* by this function. Also attach these frames to <l> frame list.
- * Return 1 if succeeded, 0 if not.
+ * Return 1 if at least one ack-eleciting frame could be built, 0 if not.
*/
static inline int qc_build_frms(struct list *outlist, struct list *inlist,
size_t room, size_t *len, size_t headlen,
@@ -5468,6 +5468,11 @@
TRACE_PROTO("************** frames build (headlen)",
QUIC_EV_CONN_BCFRMS, qc, &headlen);
+
+ /* NOTE: switch/case block inside a loop, a successful status must be
+ * returned by this function only if at least one frame could be built
+ * in the switch/case block.
+ */
list_for_each_entry_safe(cf, cfbak, inlist, list) {
/* header length, data length, frame length. */
size_t hlen, dlen, dlen_sz, avail_room, flen;
@@ -5486,7 +5491,7 @@
TRACE_PROTO(" CRYPTO data length (hlen, crypto.len, dlen)",
QUIC_EV_CONN_BCFRMS, qc, &hlen, &cf->crypto.len, &dlen);
if (!dlen)
- break;
+ continue;
/* CRYPTO frame length. */
flen = hlen + quic_int_getsize(dlen) + dlen;
@@ -5508,7 +5513,7 @@
new_cf = pool_zalloc(pool_head_quic_frame);
if (!new_cf) {
TRACE_PROTO("No memory for new crypto frame", QUIC_EV_CONN_BCFRMS, qc);
- return 0;
+ continue;
}
LIST_INIT(&new_cf->reflist);
@@ -5535,11 +5540,11 @@
((cf->type & QUIC_STREAM_FRAME_TYPE_OFF_BIT) ? quic_int_getsize(cf->stream.offset.key) : 0);
/* Compute the data length of this STREAM frame. */
avail_room = room - hlen - *len;
- if ((ssize_t)avail_room <= 0)
- break;
-
TRACE_PROTO(" New STREAM frame build (room, len)",
QUIC_EV_CONN_BCFRMS, qc, &room, len);
+ if ((ssize_t)avail_room <= 0)
+ continue;
+
if (cf->type & QUIC_STREAM_FRAME_TYPE_LEN_BIT) {
dlen = max_available_room(avail_room, &dlen_sz);
if (dlen > cf->stream.len) {
@@ -5584,7 +5589,7 @@
new_cf = pool_zalloc(pool_head_quic_frame);
if (!new_cf) {
TRACE_PROTO("No memory for new STREAM frame", QUIC_EV_CONN_BCFRMS, qc);
- return 0;
+ continue;
}
LIST_INIT(&new_cf->reflist);
@@ -5643,6 +5648,8 @@
LIST_APPEND(outlist, &cf->list);
break;
}
+
+ /* Successful status as soon as a frame could be built */
ret = 1;
}