From 4ccf3a31f596b1055d9f128e45d0a647d59b6f53 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Mon, 30 Jun 2025 09:07:54 +0200 Subject: [PATCH] ngtcp2: fix coverity warning about result handling Closes #17783 --- lib/cf-h2-proxy.c | 24 ++++++------------------ lib/easy.c | 10 +++------- lib/http.c | 5 ++--- lib/multi.c | 6 ++---- lib/url.c | 14 ++++++++++++++ lib/url.h | 13 +++++++++++++ lib/vquic/curl_ngtcp2.c | 31 ++++++++++++------------------- lib/vquic/curl_osslq.c | 30 ++++++++++-------------------- lib/vquic/curl_quiche.c | 14 ++++---------- 9 files changed, 66 insertions(+), 81 deletions(-) diff --git a/lib/cf-h2-proxy.c b/lib/cf-h2-proxy.c index 59322aa9ac..c55467c705 100644 --- a/lib/cf-h2-proxy.c +++ b/lib/cf-h2-proxy.c @@ -28,6 +28,7 @@ #include #include "urldata.h" +#include "url.h" #include "cfilters.h" #include "connect.h" #include "curl_trc.h" @@ -1315,7 +1316,7 @@ static CURLcode cf_h2_proxy_recv(struct Curl_cfilter *cf, { struct cf_h2_proxy_ctx *ctx = cf->ctx; struct cf_call_data save; - CURLcode result, r2; + CURLcode result; *pnread = 0; CF_DATA_SAVE(save, cf, data); @@ -1339,9 +1340,7 @@ static CURLcode cf_h2_proxy_recv(struct Curl_cfilter *cf, nghttp2_session_consume(ctx->h2, ctx->tunnel.stream_id, *pnread); } - r2 = proxy_h2_progress_egress(cf, data); - if(r2 && (r2 != CURLE_AGAIN)) - result = r2; + result = Curl_1st_fatal(result, proxy_h2_progress_egress(cf, data)); out: if(!Curl_bufq_is_empty(&ctx->tunnel.recvbuf) && @@ -1364,7 +1363,7 @@ static CURLcode cf_h2_proxy_send(struct Curl_cfilter *cf, struct cf_h2_proxy_ctx *ctx = cf->ctx; struct cf_call_data save; int rv; - CURLcode result, r2; + CURLcode result; (void)eos; *pnwritten = 0; @@ -1394,19 +1393,8 @@ static CURLcode cf_h2_proxy_send(struct Curl_cfilter *cf, } } - r2 = proxy_h2_progress_ingress(cf, data); - if(r2 && (r2 != CURLE_AGAIN)) { - result = r2; - goto out; - } - - /* Call the nghttp2 send loop and flush to write ALL buffered data, - * headers and/or request body completely out to the network */ - r2 = proxy_h2_progress_egress(cf, data); - if(r2 && (r2 != CURLE_AGAIN)) { - result = r2; - goto out; - } + result = Curl_1st_fatal(result, proxy_h2_progress_ingress(cf, data)); + result = Curl_1st_fatal(result, proxy_h2_progress_egress(cf, data)); if(!result && proxy_h2_should_close_session(ctx)) { /* nghttp2 thinks this session is done. If the stream has not been diff --git a/lib/easy.c b/lib/easy.c index d540888d3a..99cbd970ea 100644 --- a/lib/easy.c +++ b/lib/easy.c @@ -1129,7 +1129,7 @@ void curl_easy_reset(CURL *d) */ CURLcode curl_easy_pause(CURL *d, int action) { - CURLcode result = CURLE_OK, r2; + CURLcode result = CURLE_OK; bool recursive = FALSE; bool changed = FALSE; struct Curl_easy *data = d; @@ -1150,16 +1150,12 @@ CURLcode curl_easy_pause(CURL *d, int action) if(send_paused != send_paused_new) { changed = TRUE; - r2 = Curl_xfer_pause_send(data, send_paused_new); - if(r2) - result = r2; + result = Curl_1st_err(result, Curl_xfer_pause_send(data, send_paused_new)); } if(recv_paused != recv_paused_new) { changed = TRUE; - r2 = Curl_xfer_pause_recv(data, recv_paused_new); - if(r2) - result = r2; + result = Curl_1st_err(result, Curl_xfer_pause_recv(data, recv_paused_new)); } /* If not completely pausing both directions now, run again in any case. */ diff --git a/lib/http.c b/lib/http.c index 2dfb631fd3..32e54c2683 100644 --- a/lib/http.c +++ b/lib/http.c @@ -3962,9 +3962,8 @@ static CURLcode http_on_response(struct Curl_easy *data, out: if(last_hd) { /* if not written yet, write it now */ - CURLcode r2 = http_write_header(data, last_hd, last_hd_len); - if(!result) - result = r2; + result = Curl_1st_err( + result, http_write_header(data, last_hd, last_hd_len)); } return result; } diff --git a/lib/multi.c b/lib/multi.c index 86afc60016..fe2cca4039 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -621,7 +621,7 @@ static CURLcode multi_done(struct Curl_easy *data, after an error was detected */ bool premature) { - CURLcode result, r2; + CURLcode result; struct connectdata *conn = data->conn; struct multi_done_ctx mdctx; @@ -670,9 +670,7 @@ static CURLcode multi_done(struct Curl_easy *data, } /* Make sure that transfer client writes are really done now. */ - r2 = Curl_xfer_write_done(data, premature); - if(r2 && !result) - result = r2; + result = Curl_1st_err(result, Curl_xfer_write_done(data, premature)); /* Inform connection filters that this transfer is done */ Curl_conn_ev_data_done(data, premature); diff --git a/lib/url.c b/lib/url.c index 977b6a6049..1f73c1e3ac 100644 --- a/lib/url.c +++ b/lib/url.c @@ -4101,3 +4101,17 @@ void *Curl_conn_meta_get(struct connectdata *conn, const char *key) { return Curl_hash_pick(&conn->meta_hash, CURL_UNCONST(key), strlen(key) + 1); } + +CURLcode Curl_1st_err(CURLcode r1, CURLcode r2) +{ + return r1 ? r1 : r2; +} + +CURLcode Curl_1st_fatal(CURLcode r1, CURLcode r2) +{ + if(r1 && (r1 != CURLE_AGAIN)) + return r1; + if(r2 && (r2 != CURLE_AGAIN)) + return r2; + return r1; +} diff --git a/lib/url.h b/lib/url.h index 7aba98dbb9..bd4060c884 100644 --- a/lib/url.h +++ b/lib/url.h @@ -101,6 +101,19 @@ CURLcode Curl_conn_upkeep(struct Curl_easy *data, struct connectdata *conn, struct curltime *now); +/** + * Always eval all arguments, return the first result != CURLE_OK. + * A non-short-circuit evaluation. + */ +CURLcode Curl_1st_err(CURLcode r1, CURLcode r2); + +/** + * Always eval all arguments, return the first + * result != (CURLE_OK|CURLE_AGAIN) or `r1`. + * A non-short-circuit evaluation. + */ +CURLcode Curl_1st_fatal(CURLcode r1, CURLcode r2); + #if defined(USE_HTTP2) || defined(USE_HTTP3) void Curl_data_priority_clear_state(struct Curl_easy *data); #else diff --git a/lib/vquic/curl_ngtcp2.c b/lib/vquic/curl_ngtcp2.c index 5d40fd5ccf..c534e905f6 100644 --- a/lib/vquic/curl_ngtcp2.c +++ b/lib/vquic/curl_ngtcp2.c @@ -47,6 +47,7 @@ #endif #include "../urldata.h" +#include "../url.h" #include "../uint-hash.h" #include "../sendf.h" #include "../strdup.h" @@ -1325,15 +1326,11 @@ static CURLcode cf_ngtcp2_recv(struct Curl_cfilter *cf, struct Curl_easy *data, result = CURLE_AGAIN; out: - if(cf_progress_egress(cf, data, &pktx)) { - result = CURLE_SEND_ERROR; - } - else { - CURLcode r2 = check_and_set_expiry(cf, data, &pktx); - if(r2) - result = r2; - } + result = Curl_1st_err(result, cf_progress_egress(cf, data, &pktx)); + result = Curl_1st_err(result, check_and_set_expiry(cf, data, &pktx)); + CURL_TRC_CF(data, cf, "[%" FMT_PRId64 "] cf_recv(blen=%zu) -> %dm, %zu", + stream ? stream->id : -1, blen, result, *pnread); CF_DATA_RESTORE(cf, save); return result; @@ -1581,7 +1578,7 @@ static CURLcode cf_ngtcp2_send(struct Curl_cfilter *cf, struct Curl_easy *data, struct h3_stream_ctx *stream = H3_STREAM_CTX(ctx, data); struct cf_call_data save; struct pkt_io_ctx pktx; - CURLcode result = CURLE_OK, r2; + CURLcode result = CURLE_OK; CF_DATA_SAVE(save, cf, data); DEBUGASSERT(cf->connected); @@ -1595,10 +1592,9 @@ static CURLcode cf_ngtcp2_send(struct Curl_cfilter *cf, struct Curl_easy *data, return ctx->tls_vrfy_result; (void)eos; /* use for stream EOF and block handling */ - r2 = cf_progress_ingress(cf, data, &pktx); - if(r2) { - result = r2; - } + result = cf_progress_ingress(cf, data, &pktx); + if(result) + goto out; if(!stream || stream->id < 0) { if(ctx->shutdown_started) { @@ -1655,14 +1651,11 @@ static CURLcode cf_ngtcp2_send(struct Curl_cfilter *cf, struct Curl_easy *data, if(*pnwritten > 0 && !ctx->tls_handshake_complete && ctx->use_earlydata) ctx->earlydata_skip += *pnwritten; - r2 = cf_progress_egress(cf, data, &pktx); - if(r2) - result = r2; + DEBUGASSERT(!result); + result = cf_progress_egress(cf, data, &pktx); out: - r2 = check_and_set_expiry(cf, data, &pktx); - if(r2) - result = r2; + result = Curl_1st_err(result, check_and_set_expiry(cf, data, &pktx)); CURL_TRC_CF(data, cf, "[%" FMT_PRId64 "] cf_send(len=%zu) -> %d, %zu", stream ? stream->id : -1, len, result, *pnwritten); diff --git a/lib/vquic/curl_osslq.c b/lib/vquic/curl_osslq.c index d435d7f512..8e6f8c11a6 100644 --- a/lib/vquic/curl_osslq.c +++ b/lib/vquic/curl_osslq.c @@ -1995,7 +1995,7 @@ static CURLcode cf_osslq_send(struct Curl_cfilter *cf, struct Curl_easy *data, struct h3_stream_ctx *stream = H3_STREAM_CTX(ctx, data); struct cf_call_data save; ssize_t nwritten; - CURLcode result = CURLE_OK, r2; + CURLcode result = CURLE_OK; (void)eos; /* use to end stream */ CF_DATA_SAVE(save, cf, data); @@ -2049,14 +2049,11 @@ static CURLcode cf_osslq_send(struct Curl_cfilter *cf, struct Curl_easy *data, (void)nghttp3_conn_resume_stream(ctx->h3.conn, stream->s.id); } - r2 = cf_progress_egress(cf, data); - if(r2) - result = r2; + result = Curl_1st_err(result, cf_progress_egress(cf, data)); out: - r2 = check_and_set_expiry(cf, data); - if(r2) - result = r2; + result = Curl_1st_err(result, check_and_set_expiry(cf, data)); + CURL_TRC_CF(data, cf, "[%" FMT_PRId64 "] cf_send(len=%zu) -> %d, %zu", stream ? stream->s.id : -1, len, result, *pnwritten); CF_DATA_RESTORE(cf, save); @@ -2093,7 +2090,7 @@ static CURLcode cf_osslq_recv(struct Curl_cfilter *cf, struct Curl_easy *data, struct cf_osslq_ctx *ctx = cf->ctx; struct h3_stream_ctx *stream = H3_STREAM_CTX(ctx, data); struct cf_call_data save; - CURLcode result = CURLE_OK, r2; + CURLcode result = CURLE_OK; (void)ctx; CF_DATA_SAVE(save, cf, data); @@ -2117,11 +2114,9 @@ static CURLcode cf_osslq_recv(struct Curl_cfilter *cf, struct Curl_easy *data, } } - r2 = cf_progress_ingress(cf, data); - if(r2) { - result = r2; + result = Curl_1st_err(result, cf_progress_ingress(cf, data)); + if(result) goto out; - } /* recvbuf had nothing before, maybe after progressing ingress? */ if(!*pnread && !Curl_bufq_is_empty(&stream->recvbuf)) { @@ -2145,14 +2140,9 @@ static CURLcode cf_osslq_recv(struct Curl_cfilter *cf, struct Curl_easy *data, } out: - if(cf_progress_egress(cf, data)) { - result = CURLE_SEND_ERROR; - } - else { - r2 = check_and_set_expiry(cf, data); - if(r2) - result = r2; - } + result = Curl_1st_err(result, cf_progress_egress(cf, data)); + result = Curl_1st_err(result, check_and_set_expiry(cf, data)); + CURL_TRC_CF(data, cf, "[%" FMT_PRId64 "] cf_recv(len=%zu) -> %d, %zu", stream ? stream->s.id : -1, len, result, *pnread); CF_DATA_RESTORE(cf, save); diff --git a/lib/vquic/curl_quiche.c b/lib/vquic/curl_quiche.c index 6d79842fba..912f312a1f 100644 --- a/lib/vquic/curl_quiche.c +++ b/lib/vquic/curl_quiche.c @@ -848,7 +848,7 @@ static CURLcode cf_quiche_recv(struct Curl_cfilter *cf, struct Curl_easy *data, { struct cf_quiche_ctx *ctx = cf->ctx; struct h3_stream_ctx *stream = H3_STREAM_CTX(ctx, data); - CURLcode result = CURLE_OK, r2; + CURLcode result = CURLE_OK; *pnread = 0; vquic_ctx_update_time(&ctx->q); @@ -896,11 +896,7 @@ static CURLcode cf_quiche_recv(struct Curl_cfilter *cf, struct Curl_easy *data, } out: - r2 = cf_flush_egress(cf, data); - if(r2) { - CURL_TRC_CF(data, cf, "cf_recv, flush egress failed"); - result = r2; - } + result = Curl_1st_err(result, cf_flush_egress(cf, data)); if(*pnread > 0) ctx->data_recvd += *pnread; CURL_TRC_CF(data, cf, "[%"FMT_PRIu64"] cf_recv(total=%" @@ -1085,7 +1081,7 @@ static CURLcode cf_quiche_send(struct Curl_cfilter *cf, struct Curl_easy *data, { struct cf_quiche_ctx *ctx = cf->ctx; struct h3_stream_ctx *stream = H3_STREAM_CTX(ctx, data); - CURLcode result, r2; + CURLcode result; *pnwritten = 0; vquic_ctx_update_time(&ctx->q); @@ -1126,9 +1122,7 @@ static CURLcode cf_quiche_send(struct Curl_cfilter *cf, struct Curl_easy *data, } out: - r2 = cf_flush_egress(cf, data); - if(r2) - result = r2; + result = Curl_1st_err(result, cf_flush_egress(cf, data)); CURL_TRC_CF(data, cf, "[%" FMT_PRIu64 "] cf_send(len=%zu) -> %d, %zu", stream ? stream->id : (curl_uint64_t)~0, len, -- 2.47.3