tls_process: don't set variable that's never read
PATCH 1/2 tls_process: don't set variable that's never read
Found by the clang static analyzer: the state_change variable is set, but never read afterwards. This code has been like this since 2005, makes sense without setting state_change to true, and has worked fine for the past 11 years.
--- src/openvpn/ssl.c | 1 - 1 file changed, 1 deletion(-)
src/openvpn/ssl.c +0 -1
@@ -2809,7 +2809,6 @@ tls_process (struct tls_multi *multi, RELIABLE_ACK_SIZE, false); *to_link = buf; active = true; - state_change = true; dmsg (D_TLS_DEBUG, "Dedicated ACK -> TCP/UDP"); }#endif
Signature · 1 line
2.7.4
PATCH 2/2 Unconditionally enable TLS_AGGREGATE_ACK
This define has been enabled by default since 2005, and was not configurable through ./configure (but just by changing ssl.h). Let's get rid of it.
--- src/openvpn/ssl.c | 18 ------------------ src/openvpn/ssl.h | 9 +-------- 2 files changed, 1 insertion(+), 26 deletions(-)
src/openvpn/ssl.c +0 -18
@@ -2639,22 +2639,6 @@ tls_process (struct tls_multi *multi,break ; } -#ifndef TLS_AGGREGATE_ACK -/* Send 1 or more ACKs (each received control packet gets one ACK) */ -if (!to_link->len && !reliable_ack_empty (ks->rec_ack)) - { - buf = &ks->ack_write_buf; -ASSERT (buf_init (buf,FRAME_HEADROOM (&multi->opt.frame))); -write_control_auth (session, ks, buf, to_link_addr,P_ACK_V1 , -RELIABLE_ACK_SIZE , false); - *to_link = *buf; - active = true; - state_change = true; -dmsg (D_TLS_DEBUG ,"Dedicated ACK -> TCP/UDP" ); -break ; - } -#endif -/* Write incoming ciphertext to TLS object */ buf =reliable_get_buf_sequenced (ks->rec_reliable);if (buf)
@@ -2799,7 +2783,6 @@ tls_process (struct tls_multi *multi,update_time (); -#ifdef TLS_AGGREGATE_ACK /* Send 1 or more ACKs (each received control packet gets one ACK) */ if (!to_link->len && !reliable_ack_empty (ks->rec_ack)) {
@@ -2811,7 +2794,6 @@ tls_process (struct tls_multi *multi, active = true;dmsg (D_TLS_DEBUG ,"Dedicated ACK -> TCP/UDP" ); } -#endif /* When should we wake up again? */ {
src/openvpn/ssl.h +1 -8
@@ -70,15 +70,8 @@#define P_FIRST_OPCODE 1#define P_LAST_OPCODE 9 -/* Should we aggregate TLS -* acknowledgements, and tack them onto -* control packets? */ -#define TLS_AGGREGATE_ACK -/* -* If TLS_AGGREGATE_ACK, set the -* max number of acknowledgments that -* can "hitch a ride" on an outgoing +* Set the max number of acknowledgments that can "hitch a ride" on an outgoing * non-P_ACK_V1 control packet. */ #define CONTROL_SEND_ACK_MAX 4
Signature · 1 line
2.7.4
[Openvpn-devel] [PATCH applied] Re: Unconditionally enable TLS_AGGREGATE_ACK
ACK. If nobody ever wants the alternate branch, just kick it out.
Your patch has been applied to the master branch.
commit 718257811b42b2bf4a7ee325a38c3cb147c53cc2
Author: Steffan Karger Date: Mon Nov 28 15:53:21 2016 +0100
Unconditionally enable TLS_AGGREGATE_ACK
Signed-off-by: Steffan Karger <[email protected]...>
Acked-by: Gert Doering <[email protected]...>
Message-Id: <[email protected]...>
URL: https://www.mail-archive.com/[email protected]/msg13261.html
Signed-off-by: Gert Doering <[email protected]...>--
kind regards,
Gert Doering
ACK. Easy enough.
Your patch has been applied to the master branch.
commit 06c54466c86ee3beff9b6cd75f40de5e431d8235
Author: Steffan Karger Date: Mon Nov 28 15:53:20 2016 +0100
tls_process: don't set variable that's never read
Signed-off-by: Steffan Karger <[email protected]...>
Acked-by: Gert Doering <[email protected]...>
Message-Id: <[email protected]...>
URL: https://www.mail-archive.com/[email protected]/msg13260.html
Signed-off-by: Gert Doering <[email protected]...>--
kind regards,
Gert Doering