Re: [PATCH] selftests: net: fix resource leak in napi_id_helper.c

[Simon Horman <horms@kernel.org>]

On Wed, Jun 25, 2025 at 09:03:07PM +0530, Malaya Kumar Rout wrote:
> Resolve minor resource leaks reported by cppcheck in napi_id_helper.c
> 
> cppcheck output before this patch:
> tools/testing/selftests/drivers/net/napi_id_helper.c:37:3: error: Resource leak: server [resourceLeak]
> tools/testing/selftests/drivers/net/napi_id_helper.c:46:3: error: Resource leak: server [resourceLeak]
> tools/testing/selftests/drivers/net/napi_id_helper.c:51:3: error: Resource leak: server [resourceLeak]
> tools/testing/selftests/drivers/net/napi_id_helper.c:59:3: error: Resource leak: server [resourceLeak]
> tools/testing/selftests/drivers/net/napi_id_helper.c:67:3: error: Resource leak: server [resourceLeak]
> tools/testing/selftests/drivers/net/napi_id_helper.c:76:3: error: Resource leak: server [resourceLeak]
> 
> cppcheck output after this patch:
> No resource leaks found
> 
> Signed-off-by: Malaya Kumar Rout <malayarout91@gmail.com>

Thanks,

I agree that it is nice to address these warnings.
But for completeness doesn't client also need to be closed
if an error occurs after it is accepted?

Also, I'd suggest using a ladder of goto labels for error handling,
as is common in (Neworking?) kernel code.

E.g. (compile tested only!!)

diff --git a/tools/testing/selftests/drivers/net/napi_id_helper.c b/tools/testing/selftests/drivers/net/napi_id_helper.c
index eecd610c2109..105517dc315d 100644
--- a/tools/testing/selftests/drivers/net/napi_id_helper.c
+++ b/tools/testing/selftests/drivers/net/napi_id_helper.c
@@ -34,7 +34,7 @@ int main(int argc, char *argv[])
 
 	if (setsockopt(server, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt))) {
 		perror("setsockopt");
-		return 1;
+		goto err_close_server;
 	}
 
 	address.sin_family = AF_INET;
@@ -43,12 +43,12 @@ int main(int argc, char *argv[])
 
 	if (bind(server, (struct sockaddr *)&address, sizeof(address)) < 0) {
 		perror("bind failed");
-		return 1;
+		goto err_close_server;
 	}
 
 	if (listen(server, 1) < 0) {
 		perror("listen");
-		return 1;
+		goto err_close_server;
 	}
 
 	ksft_ready();
@@ -56,7 +56,7 @@ int main(int argc, char *argv[])
 	client = accept(server, NULL, 0);
 	if (client < 0) {
 		perror("accept");
-		return 1;
+		goto err_close_server;
 	}
 
 	optlen = sizeof(napi_id);
@@ -64,7 +64,7 @@ int main(int argc, char *argv[])
 			 &optlen);
 	if (ret != 0) {
 		perror("getsockopt");
-		return 1;
+		goto err_close_client;
 	}
 
 	read(client, buf, 1024);
@@ -73,11 +73,17 @@ int main(int argc, char *argv[])
 
 	if (napi_id == 0) {
 		fprintf(stderr, "napi ID is 0\n");
-		return 1;
+		goto err_close_client;
 	}
 
 	close(client);
 	close(server);
 
 	return 0;
+
+err_close_client:
+	close(client);
+err_close_server:
+	close(server);
+	return 1;
 }