[PATCH net v4] selftest:net: Fix uninit return values

[PATCH net v4] selftest:net: Fix uninit return values

[Sidharth Seela]

Fix functions that return undefined values. These issues were caught by
running clang using LLVM=1 option.


Clang warnings are as follows:
ovpn-cli.c:1587:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
 1587 |         if (!sock) {
      |             ^~~~~
ovpn-cli.c:1635:9: note: uninitialized use occurs here
 1635 |         return ret;
      |                ^~~
ovpn-cli.c:1587:2: note: remove the 'if' if its condition is always false
 1587 |         if (!sock) {
      |         ^~~~~~~~~~~~
 1588 |                 fprintf(stderr, "cannot allocate netlink socket\n");
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1589 |                 goto err_free;
      |                 ~~~~~~~~~~~~~~
 1590 |         }
      |         ~
ovpn-cli.c:1584:15: note: initialize the variable 'ret' to silence this warning
 1584 |         int mcid, ret;
      |                      ^
      |                       = 0
ovpn-cli.c:2107:7: warning: variable 'ret' is used uninitialized whenever switch case is taken [-Wsometimes-uninitialized]
 2107 |         case CMD_INVALID:
      |              ^~~~~~~~~~~
ovpn-cli.c:2111:9: note: uninitialized use occurs here
 2111 |         return ret;
      |                ^~~
ovpn-cli.c:1939:12: note: initialize the variable 'ret' to silence this warning
 1939 |         int n, ret;
      |                   ^
      |


Fixes: 959bc330a439 ("testing/selftests: add test tool and scripts for ovpn module")
ovpn module")
Signed-off-by: Sidharth Seela <sidharthseela@gmail.com>
---
v4:
	- Move changelog below sign-off.
	- Remove double-hyphens in commit description.
v3:
	- Use prefix net.
	- Remove so_txtime fix as default case calls error().
	- Changelog before sign-off.
	- Three dashes after sign-off
v2:
	- Use subsystem name "net".
	- Add fixes tags.
	- Remove txtimestamp fix as default case calls error.
	- Assign constant error string instead of NULL.


diff --git a/tools/testing/selftests/net/ovpn/ovpn-cli.c b/tools/testing/selftests/net/ovpn/ovpn-cli.c
index 9201f2905f2c..20d00378f34a 100644
--- a/tools/testing/selftests/net/ovpn/ovpn-cli.c
+++ b/tools/testing/selftests/net/ovpn/ovpn-cli.c
@@ -1581,7 +1581,7 @@ static int ovpn_listen_mcast(void)
 {
 	struct nl_sock *sock;
 	struct nl_cb *cb;
-	int mcid, ret;
+	int mcid, ret = -1;
 
 	sock = nl_socket_alloc();
 	if (!sock) {
@@ -1936,7 +1936,7 @@ static int ovpn_run_cmd(struct ovpn_ctx *ovpn)
 {
 	char peer_id[10], vpnip[INET6_ADDRSTRLEN], laddr[128], lport[10];
 	char raddr[128], rport[10];
-	int n, ret;
+	int n, ret = -1;
 	FILE *fp;
 
 	switch (ovpn->cmd) {
-- 
2.47.3

Re: [PATCH net v4] selftest:net: Fix uninit return values

[Antonio Quartulli]

Hi,

Thanks a lot for fixing this - I hadn't see the warnings with gcc.

On 30/09/2025 12:06, Sidharth Seela wrote:
> Fix functions that return undefined values. These issues were caught by
> running clang using LLVM=1 option.
> 
> 
> Clang warnings are as follows:
> ovpn-cli.c:1587:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>   1587 |         if (!sock) {
>        |             ^~~~~
> ovpn-cli.c:1635:9: note: uninitialized use occurs here
>   1635 |         return ret;
>        |                ^~~
> ovpn-cli.c:1587:2: note: remove the 'if' if its condition is always false
>   1587 |         if (!sock) {
>        |         ^~~~~~~~~~~~
>   1588 |                 fprintf(stderr, "cannot allocate netlink socket\n");
>        |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   1589 |                 goto err_free;
>        |                 ~~~~~~~~~~~~~~
>   1590 |         }
>        |         ~
> ovpn-cli.c:1584:15: note: initialize the variable 'ret' to silence this warning
>   1584 |         int mcid, ret;
>        |                      ^
>        |                       = 0
> ovpn-cli.c:2107:7: warning: variable 'ret' is used uninitialized whenever switch case is taken [-Wsometimes-uninitialized]
>   2107 |         case CMD_INVALID:
>        |              ^~~~~~~~~~~
> ovpn-cli.c:2111:9: note: uninitialized use occurs here
>   2111 |         return ret;
>        |                ^~~
> ovpn-cli.c:1939:12: note: initialize the variable 'ret' to silence this warning
>   1939 |         int n, ret;
>        |                   ^
>        |
> 
> 
> Fixes: 959bc330a439 ("testing/selftests: add test tool and scripts for ovpn module")
> ovpn module")
> Signed-off-by: Sidharth Seela <sidharthseela@gmail.com>
> ---
> v4:
> 	- Move changelog below sign-off.
> 	- Remove double-hyphens in commit description.
> v3:
> 	- Use prefix net.
> 	- Remove so_txtime fix as default case calls error().
> 	- Changelog before sign-off.
> 	- Three dashes after sign-off
> v2:
> 	- Use subsystem name "net".
> 	- Add fixes tags.
> 	- Remove txtimestamp fix as default case calls error.
> 	- Assign constant error string instead of NULL.
> 
> 
> diff --git a/tools/testing/selftests/net/ovpn/ovpn-cli.c b/tools/testing/selftests/net/ovpn/ovpn-cli.c
> index 9201f2905f2c..20d00378f34a 100644
> --- a/tools/testing/selftests/net/ovpn/ovpn-cli.c
> +++ b/tools/testing/selftests/net/ovpn/ovpn-cli.c
> @@ -1581,7 +1581,7 @@ static int ovpn_listen_mcast(void)
>   {
>   	struct nl_sock *sock;
>   	struct nl_cb *cb;
> -	int mcid, ret;
> +	int mcid, ret = -1;

ret goes uninitialized only under the "if (!sock)" condition, therefore 
I'd rather assign ret a meaningful value instead of -1.

How about adding "err = -ENOMEM;" directly inside the if block?

>   
>   	sock = nl_socket_alloc();
>   	if (!sock) {
> @@ -1936,7 +1936,7 @@ static int ovpn_run_cmd(struct ovpn_ctx *ovpn)
>   {
>   	char peer_id[10], vpnip[INET6_ADDRSTRLEN], laddr[128], lport[10];
>   	char raddr[128], rport[10];
> -	int n, ret;
> +	int n, ret = -1;

Same here.
ret goes uninitialized only under the "CMD_INVALID" case.

How about adding "ret = -EINVAL;" inside the affected case?


Both values are returned by ovpn_run_cmd() and then printed as 
strerror(-ret).
If we blindly use -1 we will get "Operation not permitted" which will 
confuse the user IMHO.

Thanks a lot!

>   	FILE *fp;
>   
>   	switch (ovpn->cmd) {

-- 
Antonio Quartulli
OpenVPN Inc.

Re: [PATCH net v4] selftest:net: Fix uninit return values

[Sidharth Seela]

On Tue, Sep 30, 2025 at 4:50 PM Antonio Quartulli <antonio@openvpn.net> wrote:
> Hi,
> Thanks a lot for fixing this - I hadn't see the warnings with gcc.

I am glad, thankyou.

> ret goes uninitialized only under the "if (!sock)" condition, therefore
> I'd rather assign ret a meaningful value instead of -1.

Yes, you are right.

> How about adding "err = -ENOMEM;" directly inside the if block?
> Same here.
> ret goes uninitialized only under the "CMD_INVALID" case.
> How about adding "ret = -EINVAL;" inside the affected case?
> Both values are returned by ovpn_run_cmd() and then printed as
> strerror(-ret).
> If we blindly use -1 we will get "Operation not permitted" which will
> confuse the user IMHO.

Alright, understood, Thank you.

Sending in the changes in v5.


-- 
Thanks,
Sidharth Seela
www.realtimedesign.org