diff mbox series

[Openvpn-devel,3/3] ssl: remove unneeded if block

Message ID 20210405080007.1665-3-a@unstable.cc
State Changes Requested
Headers show
Series [Openvpn-devel,1/3] openssl: fix EVP_PKEY_CTX memory leak | expand

Commit Message

Antonio Quartulli April 4, 2021, 10 p.m. UTC 
From: Antonio Quartulli <antonio@openvpn.net>

There is no need to check the result of a boolean function and then
assign a constant value to a variable based on that check.

Directly assign the return value of the function to the variable.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 src/openvpn/ssl.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Gert Doering April 5, 2021, 12:38 a.m. UTC | #1 
Hi,

On Mon, Apr 05, 2021 at 10:00:07AM +0200, Antonio Quartulli wrote:
>      /* compute PRF */
> -    if (!ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, output, output_len))
> -    {
> -        ret = false;
> -    }
> +    ret = ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, output,
> +                       output_len);

If touching this, I would move the "bool ret" declaration to here as
well.  Doesn't make sense to have a "bool ret = true" declaration 15
lines further up if the first use is "assign it a new value", then.

Thus,

+    bool ret = ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len,
+                            output, output_len);

gert
Antonio Quartulli April 5, 2021, 2:24 a.m. UTC | #2 
Hi,

On 05/04/2021 12:38, Gert Doering wrote:
> Hi,
> 
> On Mon, Apr 05, 2021 at 10:00:07AM +0200, Antonio Quartulli wrote:
>>      /* compute PRF */
>> -    if (!ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, output, output_len))
>> -    {
>> -        ret = false;
>> -    }
>> +    ret = ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, output,
>> +                       output_len);
> 
> If touching this, I would move the "bool ret" declaration to here as
> well.  Doesn't make sense to have a "bool ret = true" declaration 15
> lines further up if the first use is "assign it a new value", then.
> 
> Thus,
> 
> +    bool ret = ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len,
> +                            output, output_len);
> 

Agreed. v2 will land soon.

Regards,
diff mbox series

Patch

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 9d18c6e5..cb2a3e82 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1614,10 +1614,8 @@  openvpn_PRF(const uint8_t *secret,
     }
 
     /* compute PRF */
-    if (!ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, output, output_len))
-    {
-        ret = false;
-    }
+    ret = ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, output,
+                       output_len);
 
     buf_clear(&seed);
     free_buf(&seed);