| Message ID | 20210331155508.19423-1-arne@rfc2549.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [Openvpn-devel] Fix binary and used instead auth-token check instead of logical and | expand |
Hi, I would reword the commit subject, because the way it is now fails to highlight that we are talking about a bitwise-and operator (&). On 31/03/2021 17:55, Arne Schwabe wrote: > AUTH_TOKEN_HMAC_OK is 1, so the first term is always 0/1 and the bool > from the second part is also 0/1, so the & does the same in this instance > as &&. > > In this specific case & instead && does not change behaviour but using > && is the intended semantic behaviour. > > Signed-off-by: Arne Schwabe <arne@rfc2549.org> > --- > src/openvpn/auth_token.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c > index cc70c06c3..d571b686e 100644 > --- a/src/openvpn/auth_token.c > +++ b/src/openvpn/auth_token.c > @@ -99,7 +99,7 @@ add_session_token_env(struct tls_session *session, struct tls_multi *multi, > /* We had a valid session id before */ > const char *session_id_source; > if (multi->auth_token_state_flags & AUTH_TOKEN_HMAC_OK > - &!(multi->auth_token_state_flags & AUTH_TOKEN_EXPIRED)) > + && !(multi->auth_token_state_flags & AUTH_TOKEN_EXPIRED)) > { > session_id_source = up->password; > } > Compile-tested and I stared at the change long enough that I can feel confident it does not change any functional behaviour. The commit subject did not pass the compile test :) therefore I Will wait for a new revision before leaving my ACK. Cheers,
Acked-by: Gert Doering <gert@greenie.muc.de> Your patch has been applied to the master branch. Release/2.5 has this fix already, as part of commit 3aca477a1b58. Antonio complained about the patch subject:, and asked for a v2 - which I did not see anywhere, so I rewrote the commit subject and now it's out of my work queue :-) commit 0cbfa10e6aac01831bebe42ab33dc56c4704c1a6 Author: Arne Schwabe Date: Wed Mar 31 17:55:08 2021 +0200 Fix binary and (&) used in auth-token check instead of logical and (&&) Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de> Message-Id: <20210331155508.19423-1-arne@rfc2549.org> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21911.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c index cc70c06c3..d571b686e 100644 --- a/src/openvpn/auth_token.c +++ b/src/openvpn/auth_token.c @@ -99,7 +99,7 @@ add_session_token_env(struct tls_session *session, struct tls_multi *multi, /* We had a valid session id before */ const char *session_id_source; if (multi->auth_token_state_flags & AUTH_TOKEN_HMAC_OK - &!(multi->auth_token_state_flags & AUTH_TOKEN_EXPIRED)) + && !(multi->auth_token_state_flags & AUTH_TOKEN_EXPIRED)) { session_id_source = up->password; }
AUTH_TOKEN_HMAC_OK is 1, so the first term is always 0/1 and the bool from the second part is also 0/1, so the & does the same in this instance as &&. In this specific case & instead && does not change behaviour but using && is the intended semantic behaviour. Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/auth_token.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)