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 --- tools/testing/selftests/drivers/net/napi_id_helper.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/tools/testing/selftests/drivers/net/napi_id_helper.c b/tools/testing/selftests/drivers/net/napi_id_helper.c index eecd610c2109..1441b8d49b91 100644 --- a/tools/testing/selftests/drivers/net/napi_id_helper.c +++ b/tools/testing/selftests/drivers/net/napi_id_helper.c @@ -34,6 +34,7 @@ int main(int argc, char *argv[])
if (setsockopt(server, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt))) { perror("setsockopt"); + close(server); return 1; }
@@ -43,11 +44,13 @@ int main(int argc, char *argv[])
if (bind(server, (struct sockaddr *)&address, sizeof(address)) < 0) { perror("bind failed"); + close(server); return 1; }
if (listen(server, 1) < 0) { perror("listen"); + close(server); return 1; }
@@ -56,6 +59,7 @@ int main(int argc, char *argv[]) client = accept(server, NULL, 0); if (client < 0) { perror("accept"); + close(server); return 1; }
@@ -64,6 +68,7 @@ int main(int argc, char *argv[]) &optlen); if (ret != 0) { perror("getsockopt"); + close(server); return 1; }
@@ -73,6 +78,7 @@ int main(int argc, char *argv[])
if (napi_id == 0) { fprintf(stderr, "napi ID is 0\n"); + close(server); return 1; }
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; }
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 --- .../selftests/drivers/net/napi_id_helper.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/napi_id_helper.c b/tools/testing/selftests/drivers/net/napi_id_helper.c index eecd610c2109..47dd3291bd55 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 failure; }
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 failure; }
if (listen(server, 1) < 0) { perror("listen"); - return 1; + goto failure; }
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 failure; }
optlen = sizeof(napi_id); @@ -64,7 +64,7 @@ int main(int argc, char *argv[]) &optlen); if (ret != 0) { perror("getsockopt"); - return 1; + goto failure; }
read(client, buf, 1024); @@ -73,11 +73,18 @@ int main(int argc, char *argv[])
if (napi_id == 0) { fprintf(stderr, "napi ID is 0\n"); - return 1; + goto failure; }
close(client); close(server);
return 0; + +failure: + if (client >= 0) + close(client); + if (server >= 0) + close(server); + return 1; }
On Sat, Jun 28, 2025 at 1:19 AM Malaya Kumar Rout malayarout91@gmail.com 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
.../selftests/drivers/net/napi_id_helper.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/napi_id_helper.c b/tools/testing/selftests/drivers/net/napi_id_helper.c index eecd610c2109..47dd3291bd55 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 failure;
client variable is uninitialized at this point.
On Sat, Jun 28, 2025 at 2:15 PM Eric Dumazet edumazet@google.com wrote:
On Sat, Jun 28, 2025 at 1:19 AM Malaya Kumar Rout malayarout91@gmail.com 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
.../selftests/drivers/net/napi_id_helper.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/napi_id_helper.c b/tools/testing/selftests/drivers/net/napi_id_helper.c index eecd610c2109..47dd3291bd55 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 failure;client variable is uninitialized at this point.
I sincerely appreciate your prompt review and for identifying the variable initialization issue. If you are agreeable to initializing both the server and client to -1, I will proceed to share the updated patch.
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 --- .../selftests/drivers/net/napi_id_helper.c | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/napi_id_helper.c b/tools/testing/selftests/drivers/net/napi_id_helper.c index eecd610c2109..5581d04e180f 100644 --- a/tools/testing/selftests/drivers/net/napi_id_helper.c +++ b/tools/testing/selftests/drivers/net/napi_id_helper.c @@ -18,8 +18,8 @@ int main(int argc, char *argv[]) socklen_t optlen; char buf[1024]; int opt = 1; - int server; - int client; + int server = -1; + int client = -1; int ret;
server = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); @@ -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 failure; }
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 failure; }
if (listen(server, 1) < 0) { perror("listen"); - return 1; + goto failure; }
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 failure; }
optlen = sizeof(napi_id); @@ -64,7 +64,7 @@ int main(int argc, char *argv[]) &optlen); if (ret != 0) { perror("getsockopt"); - return 1; + goto failure; }
read(client, buf, 1024); @@ -73,11 +73,18 @@ int main(int argc, char *argv[])
if (napi_id == 0) { fprintf(stderr, "napi ID is 0\n"); - return 1; + goto failure; }
close(client); close(server);
return 0; + +failure: + if (client >= 0) + close(client); + if (server >= 0) + close(server); + return 1; }
On 6/30/25 8:36 PM, 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
Lacks fixes tag and a target tree ('net') in the subj prefix, but please do not resubmit, as there is no resource leak even without this patch as the kernel will close anyway all the open file descriptor at process exit.
/P
linux-kselftest-mirror@lists.linaro.org
-
Eric Dumazet -
Malaya Kumar Rout -
malaya kumar rout
-
Paolo Abeni -
Simon Horman