#397 more finds by Dave

[Robert McMahon]

G'day Bob,

Hope you're doing well.

I was playing with iperf2 (the 2.2.2 tree, 9 Aug build) and, mostly as an excuse to learn the tooling, I put together a little Nix-based harness that runs it through several static analyzers: cppcheck, clang-tidy, the Clang static analyzer (scan-build), GCC's -fanalyzer, and flawfinder.

As you'd expect, the vast majority of the output was noise — style nits, parser quirks, and a whole cluster of fairly alarming-looking double-free / use-after-free / NULL-dereference reports that, as far as I can tell, are false positives. They mostly sit behind the FAIL / FAIL_errno out-of-memory guards, where the analyzers don't appear to understand that thread_stop() → pthread_exit() doesn't return on the current thread.

I went through the results by hand, and only a handful looked potentially worth a second pair of eyes. I thought I'd pass them along as a friendly heads-up. There's certainly no urgency, and I may have missed some surrounding context, so please take or leave any of this.

Roughly in order of how interesting they looked to me:

src/Settings.cpp:2864 — apparently unreachable condition

if ((mExtSettings->mIntervalMode == kInterval_Time) &&
(mExtSettings->mIntervalMode <= 0)) {
Since kInterval_Time == 1, the two conditions appear to contradict each other, so I don't think the body can ever run.

I wondered whether the second condition was intended to test the interval itself:

mExtSettings->mInterval <= 0
(which, given the unsigned field, effectively means == 0).

If I've understood the surrounding logic correctly, the current result would be that -i 0 in time mode doesn't get reset to kInterval_None as intended.

src/markov.c (~107–167) — --markov bracket parser

There were a few related things in markov_graph_init() that looked worth checking.

The allocation results for graph, tmp_bra, and the temporary row arrays appear to be used without NULL checks. There are also paths where strtok() results go directly into atoi(pos) / strlen(pos) without first checking pos.

GCC's analyzer also reports a possible heap overflow around lines 152/160/167 if a row contains more comma-separated probabilities than there are allocated columns — tmp[kx][cx], with cx apparently able to advance beyond count.

It's obviously a fairly niche, local-CLI path, so I don't think any of this is particularly exciting from an exposure point of view, but it might be worth a little defensive hardening.

src/Reports.c:88 / 102 — common_copy()

This one is minor and OOM-only. *common = calloc(...) appears to be dereferenced immediately without a FAIL guard, while the sibling initialization functions in the same file guard their allocations.

Mostly just an inconsistency I noticed rather than anything I'd consider important.

src/ReportOutputs.c:3448 — possibly uninitialized start_timebuf

cppcheck thinks there's a path where start_timebuf reaches the "start before now" warning printf without iperf_formattime() having populated it first.

I'm less confident about this one — I may simply have missed a constraint elsewhere in the control flow — so I'd put this firmly in the "perhaps worth a glance" category rather than calling it a bug.

src/timestamp.c:46 and a few format-string warnings

The usage/error line passes optopt to an fprintf() whose format string doesn't appear to contain a corresponding conversion specifier — perhaps a %c went missing at some point.

There are also a handful of %d conversions used with unsigned values elsewhere. Nothing consequential that I could see; mostly just tidy-ups.

Of those, #1 looked like the clearest candidate for a small fix, while #2 seemed the most worthwhile place for some defensive checks. The others struck me as low-priority cleanup unless you happen to be working in those areas anyway.

I'm happy to turn any of these into proper patches/MRs if that would be useful — just say the word. This was mostly me having fun learning what the different analyzers do and don't catch, so please treat it as a friendly FYI rather than a review of the code.

I assume you will be able to feed this to your LLM and quickly address these tiny items

Regards,

Dave Seddon