Hi,
On 17/02/2020 09:29, Lev Stipakov wrote:
> From: Lev Stipakov <l...@openvpn.net>
>
> - remove unneeded _orig variable which stores pointer
> to tokenized string. Unlike strsep(), strtok() doesn't modify
> pointer itself, only string, so it is safe to call free() on that pointer.
>
this makes sense. We can just get rid of tmp_ciphers_orig and directly
free tmp_ciphers.
> - "token" points to content inside tokenized string. It becomes
> dangling pointer after freeing that string. Check its value before
> free() call.
To me this change is not necessary and the reason behind it is wrong.
(It is also ugly from the style perspective :-P)
>
> Signed-off-by: Lev Stipakov <l...@openvpn.net>
> ---
> src/openvpn/ssl.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
> index e708fc93..8cba3a76 100644
> --- a/src/openvpn/ssl.c
> +++ b/src/openvpn/ssl.c
> @@ -1918,7 +1918,6 @@ bool
> tls_item_in_cipher_list(const char *item, const char *list)
> {
> char *tmp_ciphers = string_alloc(list, NULL);
> - char *tmp_ciphers_orig = tmp_ciphers;
>
> const char *token = strtok(tmp_ciphers, ":");
> while (token)
> @@ -1929,9 +1928,11 @@ tls_item_in_cipher_list(const char *item, const char
> *list)
> }
> token = strtok(NULL, ":");
> }
> - free(tmp_ciphers_orig);
>
> - return token != NULL;
This expression simply checks that token contains a value different from
NULL. It's like a simple "a != 0" - it doesn't matter where that address
points to, because the pointer is not dereferenced.
> + bool found = token != NULL;
> + free(tmp_ciphers);
> +
> + return found;
For the reason above I think this change is useless (And even a bit ugly :p)
NAK.
In the future I suggest splitting unrelated changes like these in
different patches.
Assuming we had merged both, at some point of them might need to be
reverted (for whatever reason and due to both fixes being in one patch,
we would not be able to do that.
Cheers,
> }
>
> void
>
--
Antonio Quartulli
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
