netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] selftests: mptcp: initialize raw_addr to Null
@ 2025-11-26 16:30 Ankit Khushwaha
  2025-11-26 16:55 ` Matthieu Baerts
  0 siblings, 1 reply; 5+ messages in thread
From: Ankit Khushwaha @ 2025-11-26 16:30 UTC (permalink / raw)
  To: Matthieu Baerts, Mat Martineau, Geliang Tang, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt
  Cc: netdev, mptcp, linux-kselftest, linux-kernel, llvm, Shuah Khan,
	Ankit Khushwaha

The "void *raw_addr" is left uninitialized when else path is
followed raising below warning.

mptcp_connect.c:1262:11: warning: variable 'raw_addr' is used
      uninitialized whenever 'if' condition is false
      [-Wsometimes-uninitialized]

so the fix is to assign *raw_addr to NULL to suppress the warning.

Signed-off-by: Ankit Khushwaha <ankitkhushwaha.linux@gmail.com>
---
compiler used: clang version 21.1.5 (Fedora 21.1.5-1.fc43).
compilation cmd used:
	make -C tools/testing/selftests/net/mptcp CC=clang V=1 -j8

this maybe also be false positive. But somehow clang - 21.1.5
triggering this.

---
 tools/testing/selftests/net/mptcp/mptcp_connect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index 404a77bf366a..cdb81e0d08ad 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -1248,8 +1248,8 @@ void xdisconnect(int fd)
 {
 	socklen_t addrlen = sizeof(struct sockaddr_storage);
 	struct sockaddr_storage addr, empty;
+	void *raw_addr = NULL;
 	int msec_sleep = 10;
-	void *raw_addr;
 	int i, cmdlen;
 	char cmd[128];

--
2.52.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] selftests: mptcp: initialize raw_addr to Null
  2025-11-26 16:30 [PATCH] selftests: mptcp: initialize raw_addr to Null Ankit Khushwaha
@ 2025-11-26 16:55 ` Matthieu Baerts
  2025-11-28  1:00   ` Jakub Kicinski
  0 siblings, 1 reply; 5+ messages in thread
From: Matthieu Baerts @ 2025-11-26 16:55 UTC (permalink / raw)
  To: Ankit Khushwaha
  Cc: netdev, mptcp, linux-kselftest, linux-kernel, llvm, Shuah Khan,
	Mat Martineau, Geliang Tang, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt

Hi Ankit,

On 26/11/2025 17:30, Ankit Khushwaha wrote:
> The "void *raw_addr" is left uninitialized when else path is
> followed raising below warning.
> 
> mptcp_connect.c:1262:11: warning: variable 'raw_addr' is used
>       uninitialized whenever 'if' condition is false
>       [-Wsometimes-uninitialized]
> 
> so the fix is to assign *raw_addr to NULL to suppress the warning.

Thank you for having shared this patch!
> Signed-off-by: Ankit Khushwaha <ankitkhushwaha.linux@gmail.com>
> ---
> compiler used: clang version 21.1.5 (Fedora 21.1.5-1.fc43).
> compilation cmd used:
> 	make -C tools/testing/selftests/net/mptcp CC=clang V=1 -j8
> 
> this maybe also be false positive. But somehow clang - 21.1.5
> triggering this.

I confirm this, I can reproduce the warning with Clang 21.

It is indeed a false positive, because the code does that:


  if (addr.ss_family == AF_INET)
          raw_addr = &(((struct sockaddr_in *)&addr)->sin_addr);
  else if (addr.ss_family == AF_INET6)
          raw_addr = &(((struct sockaddr_in6 *)&addr)->sin6_addr);
  else
          xerror("bad family");


"xerror()" calls "exit(1)", so "raw_addr" is never used uninitialized.

I'm not sure why Clang 21 reports that now, and not before, but well,
the modification you did in the selftests doesn't hurt:

Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>


@Net maintainers: this patch can be applied in 'net' directly.


@Antik: next time, please specify the target (net/net-next) in the
subject, see [1]. No need to send a new version (except if the Net
maintainers prefer), this patch can be applied on top of "net" without
conflicts.

[1] https://docs.kernel.org/process/maintainer-netdev.html

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] selftests: mptcp: initialize raw_addr to Null
  2025-11-26 16:55 ` Matthieu Baerts
@ 2025-11-28  1:00   ` Jakub Kicinski
  2025-11-28 10:34     ` Matthieu Baerts
  0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2025-11-28  1:00 UTC (permalink / raw)
  To: Matthieu Baerts
  Cc: Ankit Khushwaha, netdev, mptcp, linux-kselftest, linux-kernel,
	llvm, Shuah Khan, Mat Martineau, Geliang Tang, David S. Miller,
	Eric Dumazet, Paolo Abeni, Simon Horman, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt

On Wed, 26 Nov 2025 17:55:14 +0100 Matthieu Baerts wrote:
> I confirm this, I can reproduce the warning with Clang 21.
> 
> It is indeed a false positive, because the code does that:
> 
> 
>   if (addr.ss_family == AF_INET)
>           raw_addr = &(((struct sockaddr_in *)&addr)->sin_addr);
>   else if (addr.ss_family == AF_INET6)
>           raw_addr = &(((struct sockaddr_in6 *)&addr)->sin6_addr);
>   else
>           xerror("bad family");
> 
> 
> "xerror()" calls "exit(1)", so "raw_addr" is never used uninitialized.
> 
> I'm not sure why Clang 21 reports that now, and not before, but well,
> the modification you did in the selftests doesn't hurt:

I think annotating xerror with __noreturn is a better fix.
Including kselftest.h will be needed.
-- 
pw-bot: cr

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] selftests: mptcp: initialize raw_addr to Null
  2025-11-28  1:00   ` Jakub Kicinski
@ 2025-11-28 10:34     ` Matthieu Baerts
  2025-11-28 18:40       ` Jakub Kicinski
  0 siblings, 1 reply; 5+ messages in thread
From: Matthieu Baerts @ 2025-11-28 10:34 UTC (permalink / raw)
  To: Jakub Kicinski, Ankit Khushwaha
  Cc: netdev, mptcp, linux-kselftest, linux-kernel, llvm, Shuah Khan,
	Mat Martineau, Geliang Tang, David S. Miller, Eric Dumazet,
	Paolo Abeni, Simon Horman, Nathan Chancellor, Nick Desaulniers,
	Bill Wendling, Justin Stitt

Hi Jakub, Ankit,

On 28/11/2025 02:00, Jakub Kicinski wrote:
> On Wed, 26 Nov 2025 17:55:14 +0100 Matthieu Baerts wrote:
>> I confirm this, I can reproduce the warning with Clang 21.
>>
>> It is indeed a false positive, because the code does that:
>>
>>
>>   if (addr.ss_family == AF_INET)
>>           raw_addr = &(((struct sockaddr_in *)&addr)->sin_addr);
>>   else if (addr.ss_family == AF_INET6)
>>           raw_addr = &(((struct sockaddr_in6 *)&addr)->sin6_addr);
>>   else
>>           xerror("bad family");
>>
>>
>> "xerror()" calls "exit(1)", so "raw_addr" is never used uninitialized.
>>
>> I'm not sure why Clang 21 reports that now, and not before, but well,
>> the modification you did in the selftests doesn't hurt:
> 
> I think annotating xerror with __noreturn is a better fix.

Good idea, I didn't know about that!

Thank you for the review!

> Including kselftest.h will be needed.

Because mptcp_connect.c is a tool that is used by other selftests, but
it doesn't interact directly with the selftests, maybe we don't need to
include it, and only add this #define in mptcp_connect.c?

  #define __noreturn __attribute__((__noreturn__))

(I don't know if a #ifndef/#endif is needed.)

That's a detail, I guess either is fine and shouldn't cause other issues.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] selftests: mptcp: initialize raw_addr to Null
  2025-11-28 10:34     ` Matthieu Baerts
@ 2025-11-28 18:40       ` Jakub Kicinski
  0 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2025-11-28 18:40 UTC (permalink / raw)
  To: Matthieu Baerts
  Cc: Ankit Khushwaha, netdev, mptcp, linux-kselftest, linux-kernel,
	llvm, Shuah Khan, Mat Martineau, Geliang Tang, David S. Miller,
	Eric Dumazet, Paolo Abeni, Simon Horman, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt

On Fri, 28 Nov 2025 11:34:38 +0100 Matthieu Baerts wrote:
> > Including kselftest.h will be needed.  
> 
> Because mptcp_connect.c is a tool that is used by other selftests, but
> it doesn't interact directly with the selftests, maybe we don't need to
> include it, and only add this #define in mptcp_connect.c?
> 
>   #define __noreturn __attribute__((__noreturn__))

Up to you, I'd be worried that some semi-automated patch generator will
send us a "cleanup" to remove this as duplication. But I'm overly
sensitive to this sort of followups so we can try if you prefer :)

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-11-28 18:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-26 16:30 [PATCH] selftests: mptcp: initialize raw_addr to Null Ankit Khushwaha
2025-11-26 16:55 ` Matthieu Baerts
2025-11-28  1:00   ` Jakub Kicinski
2025-11-28 10:34     ` Matthieu Baerts
2025-11-28 18:40       ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).