Fix for memory safety and error handling

[lokeshmuthuraj]

Overview

This PR establishes safer initialization patterns and proper error propagation in ntttcp's core infrastructure. It replaces fatal exit() calls with recoverable error codes and ensures consistent resource management patterns that subsequent PRs will build upon.

Problems Solved

1. Fatal Exit on Memory Allocation Failure

Previous behavior

default_ntttcp_test() assigned bind_address to a string literal ("0.0.0.0"), creating inconsistent memory ownership. This would cause crashes if later code tried to free it.

Root cause

The function had no way to signal allocation failures, forcing immediate program termination.

2. Memory Leak in Test Results Cleanup

How found: ASAN, Valgrind

Issue - ntttcp.c — e->results->threads array pointer never freed

free_ntttcp_test_endpoint_and_test() freed individual thread result structures but forgot to free the threads array itself.

Impact

Every test execution leaked the threads array (typically 16-64 bytes per test).

Evidence

./ntttcp -s 127.0.0.1 -t 3 -P 2 -n 1   # TCP sender, 2 ports × 1 thread = 16 bytes

Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 in malloc                   asan_malloc_linux.cpp:69
    #1 in new_ntttcp_test_endpoint ntttcp.c:160
    #2 in main                     main.c:381

SUMMARY: AddressSanitizer: 16 byte(s) leaked in 1 allocation(s).

3. Missing Socket Close Guards

How found: Static analysis, valgrind --track-fds=yes

Issue - main.c — sync socket not closed in run_ntttcp_sender() error paths

Six error paths in run_ntttcp_sender() returned ERROR_GENERAL without explicitly checking no_synch before closing synch_socket.

Impact

While not causing immediate bugs, inconsistent guard patterns made the code harder to reason about and maintain.

[lokeshmuthuraj]

@lubaihua33 / @simonxiaoss Please review the PR. Thanks.

[Copilot]

Pull request overview

This PR aims to harden ntttcp's initialization and cleanup by (1) returning an error from default_ntttcp_test instead of silently leaving bind_address as a string literal, (2) freeing the e->results->threads array that was previously leaked, and (3) explicitly closing tep->synch_socket on six error/return paths in run_ntttcp_sender. The tool version is also bumped to 1.4.4.

Changes:

• default_ntttcp_test now strdups bind_address and returns an int (NO_ERROR / ERROR_MEMORY_ALLOC); main handles the failure path.
• free_ntttcp_test_endpoint_and_test frees the e->results->threads pointer array in addition to its entries.
• run_ntttcp_sender closes synch_socket (guarded by no_synch) before each return ERROR_GENERAL and before returning from the continuous-mode sleep path.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
src/ntttcp.h	Updates default_ntttcp_test declaration to return int.
src/ntttcp.c	Drops literal bind_address assignment, strdups "0.0.0.0", returns errors, and frees e->results->threads.
src/main.c	Handles default_ntttcp_test error return and closes synch_socket on sender error/return paths.
src/const.h	Bumps TOOL_VERSION from 1.4.3 to 1.4.4.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.