Parameter parsing - memory leak fixes

[lokeshmuthuraj]

Overview

This PR establishes consistent heap-based memory ownership for bind_address throughout the parameter parsing subsystem. It fixes critical memory leaks where temporary buffer pointers and string literals were assigned to bind_address, which would later be freed, causing segmentation faults.

Depends on: PR #110
Contains changes from PR #110 but PR 110 will be merged before this one.

Problems Solved

1. Dangling Pointer in process_mappings()

How found: ASAN, static analysis

Issue - parameter.c — strdup result leaked in process_mappings()

process_mappings() calls strdup(test->mapping) and stores the result in element. strsep() then advances the element pointer. When an invalid CPU ID is detected, the function returns ERROR_ARGS without freeing the original allocation, leaking memory on every call. Even worse, there were 4 early return paths that leaked this allocation.

Impact

Memory leak on every -m flag usage (default: every test run since default mapping is "16,,").

2. String Literal Assignment

Issue

process_mappings() assigned test->bind_address = token where token was a pointer into the element buffer. This created a dangling pointer when element was freed.

Impact

Accessing test->bind_address after cleanup would read freed memory. If later freed, would cause a double-free crash.

3. IPv6 String Literal Assignment

Issue

In verify_args(), IPv6 mode unconditionally assigned test->bind_address = "::" (string literal). When cleanup tried to free this read-only memory, it caused segmentation faults.

Impact

All IPv6 tests crashed with exit code 139 (SIGSEGV).

4. Command-Line Argument Pointer Assignment

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

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

In parse_arguments(), both -r and -s assigned test->bind_address = optarg or argv[optind++]. These point into the argv array, which should not be freed.

Impact

Cleanup attempting to free argv memory would crash the program.

5. Missing Error Propagation

Issue

process_mappings() could fail but its return value was ignored in parse_arguments().

Impact

Parsing errors were silently ignored, leading to undefined behavior.

[lokeshmuthuraj]

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

[Copilot]

Pull request overview

Establishes consistent heap-based ownership of test->bind_address across the parameter parsing subsystem to eliminate memory leaks, dangling-pointer reads, and free-of-string-literal crashes (notably for IPv6 and -m/-r/-s flags). Also propagates errors from default_ntttcp_test and process_mappings, and finishes cleanup of the threads array and bind_address in test endpoint teardown.

Changes:

• process_mappings: strdup the parsed bind address (instead of pointing into a strsep buffer), check the strdup(test->mapping) allocation, and free that buffer on all (most) return paths; propagate its return value from parse_arguments (-m).
• Replace string-literal assignments to test->bind_address with strdup copies in default_ntttcp_test, verify_args (IPv6 "::"), and the -r/-s handlers; free the previous value before reassigning; free(test->bind_address) during endpoint cleanup.
• run_ntttcp_sender: close synch_socket (guarded by !no_synch) on six previously-missed error paths; bump tool version to 1.4.5; fix mixed-tab/space indentation around case 'a'.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/parameter.c	Heap-allocate bind_address in process_mappings, verify_args, and -r/-s; add strdup/allocation checks; free strsep buffer; propagate process_mappings errors.
src/ntttcp.h	Change default_ntttcp_test signature to return int to allow error propagation.
src/ntttcp.c	default_ntttcp_test returns int and strdup's "0.0.0.0"; free e->results->threads array and e->test->bind_address in endpoint cleanup.
src/main.c	Handle new error return from default_ntttcp_test; close synch_socket on six sender error paths under !no_synch.
src/const.h	Bump TOOL_VERSION from 1.4.3 to 1.4.5.

Comments suppressed due to low confidence (1)

src/main.c:363

• After this PR, test->bind_address is heap-owned (strdup'd in default_ntttcp_test, process_mappings, verify_args, and the -r/-s handlers). These error-exit paths only free(test) and leak test->bind_address. While the process is exiting, this is inconsistent with the new ownership model established in this PR and will be reported by leak sanitizers (the same tooling used to find the leaks this PR fixes). Consider freeing test->bind_address before free(test) on all three exit paths (parse_arguments failure on line 361, verify_args failure on line 369, and check_resource_limit failure on line 380).

	if (err_code != NO_ERROR) {
		PRINT_ERR("main: error when parsing args");
		print_flags(test);
		free(test);
		exit(-1);
	}

---

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