On Tue, Sep 17, 2024 at 11:54:34PM +0200, Stefano Brivio wrote:On Fri, 13 Sep 2024 14:32:07 +1000 David Gibson <david(a)gibson.dropbear.id.au> wrote:Good idea, done.This function has a block conditional on !snd_wnd_cap shortly before an #ifdef HAS_SND_WND. But snd_wnd_cap implies HAS_SND_WND (if !HAS_SND_WND, snd_wnd_cap is statically false). Therefore, simplify this down to a single conditional with an else branch. While we're there, fix some improperly indented closing braces. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- tcp.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/tcp.c b/tcp.c index cba3f3bd..6733e7e3 100644 --- a/tcp.c +++ b/tcp.c @@ -1061,27 +1061,25 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, conn->wnd_to_tap = MIN(new_wnd_to_tap >> conn->ws_to_tap, USHRT_MAX); goto out; - } - - if (!tinfo) { - if (prev_wnd_to_tap > WINDOW_DEFAULT) { - goto out; -} - tinfo = &tinfo_new; - if (getsockopt(s, SOL_TCP, TCP_INFO, tinfo, &sl)) { - goto out; -} - } - -#ifdef HAS_SND_WND - if ((conn->flags & LOCAL) || tcp_rtt_dst_low(conn)) { - new_wnd_to_tap = tinfo->tcpi_snd_wnd; } else {I thought cppcheck would report else-after-goto, but it doesn't, just else-after-return. In any case, we could simplify further by avoid that else clause (and one level of indentation) in the whole block below.It would also look more natural to me: we deal with if (!snd_wnd_cap) as a special case and go to 'out' in that special case, then we resume with the regular path. I guess this is better than the original anyway and it's not a strong preference, so I can also apply this up to 4/10 as it is (or fix up on merge). The rest of the patches up to 4/10 look good to me.Well, I've made the change now, so I might as well repost :).-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson- tcp_get_sndbuf(conn); - new_wnd_to_tap = MIN((int)tinfo->tcpi_snd_wnd, - SNDBUF_GET(conn)); + if (!tinfo) { + if (prev_wnd_to_tap > WINDOW_DEFAULT) { + goto out; + } + tinfo = &tinfo_new; + if (getsockopt(s, SOL_TCP, TCP_INFO, tinfo, &sl)) { + goto out; + } + } + + if ((conn->flags & LOCAL) || tcp_rtt_dst_low(conn)) { + new_wnd_to_tap = tinfo->tcpi_snd_wnd; + } else { + tcp_get_sndbuf(conn); + new_wnd_to_tap = MIN((int)tinfo->tcpi_snd_wnd, + SNDBUF_GET(conn)); + } } -#endif new_wnd_to_tap = MIN(new_wnd_to_tap, MAX_WINDOW); if (!(conn->events & ESTABLISHED))