iperf_parse_arguments() spuriously fails on 2nd+ call in the same process: stale i_errno from previous run's normal completion misread as new parse failure

[jerryabramson]

Summary

iperf_parse_arguments() can incorrectly return -1 on any call after the
first one in a long-lived process (e.g. a mobile app embedding libiperf
and running successive tests), because several option cases check
i_errno for "did the conversion I just did fail?" without first resetting
it — and i_errno is never cleared to 0 anywhere after the library
initializes it once at load time.

This doesn't affect the iperf3 CLI binary itself, since main.c only
calls iperf_parse_arguments() once per process. It affects any library
caller that constructs a new struct iperf_test and re-parses arguments
multiple times in one process — which is the normal, documented way to use
libiperf as an embedded library rather than spawning iperf3 as a
subprocess.

Root cause

i_errno is declared and zero-initialized once:

https://github.com/esnet/iperf/blob/3.21/src/iperf_error.c#L160

int i_errno = 0;

It is never reset anywhere else in the library. unit_atoi() /
unit_atof() / unit_atof_rate() (src/units.c) only ever set
i_errno on their own failure — they never clear it on success:

https://github.com/esnet/iperf/blob/3.21/src/units.c#L185-L222

Several iperf_parse_arguments() option cases (src/iperf_api.c) call one
of those helpers and then check if (i_errno != 0) return -1; as their
only error signal — relying on i_errno having been 0 going into the
call:

┌──────────────────────────┬────────────────────────────────────────────────────────────────────────────────────┐
│          Option          │                                        Line                                        │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ OPT_NUMSTREAMS           │ iperf_api.c#L1338 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1338) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ -b / --bitrate           │ iperf_api.c#L1359 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1359) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ OPT_SERVER_BITRATE_LIMIT │ iperf_api.c#L1378 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1378) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ -n                       │ iperf_api.c#L1394 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1394) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ -k                       │ iperf_api.c#L1401 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1401) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ -l                       │ iperf_api.c#L1408 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1408) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ -w / --window            │ iperf_api.c#L1442 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1442) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ --fq-rate                │ iperf_api.c#L1728 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1728) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ --pacing-timer           │ iperf_api.c#L1780 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1780) │
├──────────────────────────┼────────────────────────────────────────────────────────────────────────────────────┤
│ --connect-timeout        │ iperf_api.c#L1787 (https://github.com/esnet/iperf/blob/3.21/src/iperf_api.c#L1787) │
└──────────────────────────┴────────────────────────────────────────────────────────────────────────────────────┘

Separately, a normal, successful test run can itself leave i_errno set
to IESTREAMREAD (206) as an ordinary side effect of how the receive path
detects end-of-stream (observed directly via logging around
iperf_run_client() — i_errno was 206 immediately after a run that
completed and returned 0/success). That leftover value then persists into
the next call to iperf_parse_arguments() in the same process. If that
next call's argv includes any of the options above (e.g. --connect-timeout,
which is a very commonly-used option for any client), the
if (i_errno != 0) return -1; check fires immediately — before any real
parsing problem — misreporting a perfectly valid argument list as a parse
failure, with a stale/unrelated error message (whatever i_errno happened
to be, e.g. "unable to read from stream socket" from the previous run).

Note that --connect-timeout did not have this check at all in 3.19
(iperf-3.19 iperf_api.c#L1703-L1706 (https://github.com/esnet/iperf/blob/3.19/src/iperf_api.c#L1703-L1706)
— it just assigned the value with no i_errno check); the guard appears to
have been added later, which is presumably when this regression was
introduced for library callers that reuse a process across multiple runs.

Reproduction

Using libiperf directly (not the iperf3 CLI) in a single process:

1. iperf_new_test() + iperf_defaults() + iperf_parse_arguments() with
--client <host> --port <port> --connect-timeout 2000 ... and run a
normal, successful client test to completion (iperf_run_client()).
iperf_free_test().
2. Repeat step 1 again in the same process — iperf_new_test() a fresh
struct iperf_test, and call iperf_parse_arguments() with the same or
different valid arguments.
3. Step 2's iperf_parse_arguments() returns -1, even though the
arguments are valid — because i_errno was left non-zero by step 1's
normal completion, and the --connect-timeout case (or any of the other
options in the table above) treats that stale value as "the conversion I
just did failed."

Suggested fix

Reset i_errno = 0; at the start of iperf_parse_arguments(), before the
getopt_long() loop begins — mirroring the optind/optreset reset that
already exists at the end of the function for repeat-call support (#ifdef __APPLE__ block), which suggests re-entrant/repeated calls were already a
design consideration, just not fully realized for i_errno.

Environment

- iperf 3.21 (tag 3.21, commit d39cf41)
- Reproduced via a custom JNI bridge embedding libiperf in an
Android/Kotlin app (client-only build, --without-sctp --with-openssl=no)
running repeated client tests in one process — but the bug is in
iperf_parse_arguments() itself and isn't specific to that environment.

[davidBar-On]

Great analysis! I happen to have an open PR #2052 that handles similar issue about errno in the server. I therefore updated the PR to reset also i_errno, and both in iperf_reset_test() (for a server) and iperf_new_test() (for a client as described in this issue). Note that I changed iperf_new_test() instead of iperf_parse_arguments(), as I believe it fixes the problem and is more general (but of course it would help to verify that this is the case).