Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix exit code when using json output #1523

Merged
merged 3 commits into from
Jul 3, 2023

Conversation

matt9j
Copy link
Contributor

@matt9j matt9j commented May 20, 2023

  • Version of iperf3 (or development branch, such as master or
    3.1-STABLE) to which this pull request applies:

    • master
  • Issues fixed (if any):

  • Brief description of code changes (suitable for use as a commit message):

    • This PR cleans up the iperf_api::iperf_json_finish path to allow it to be called consistently on both the nominal and error case path. It contains the following 3 commits:
      • Make iperf_api::iperf_json_finish alignment self consistent
      • Make iperf_api::iperf_json_finish idempotent to simplify error path
      • Return client_api error codes, even with json output enabled

Sorry for the delay! I had a new job start up and didn't get back to this as quickly as I had hoped : ) Let me know what you think @bmah888 and @hajoha

This commit should not change behavior.
This commit adds a check to only format output and flush to the output
file if json_top is not yet null. The context of this check is not
performance senstive, since it occurs only as the program is exiting
and not on the primary data path.
Previously on error, the client API would return 0 (success) in the
cleanup and fail error handling path when outputting json to avoid
double-closing the JSON output. With iperf_json_finish now idempotent,
the client api can faithfully return an error when it fails, and the
calling context will not double-close the output if it has already
been closed.

This addresses esnet#1405 .
@matt9j matt9j force-pushed the fix-exit-code-when-doing-json-output branch from 2da06de to 060cde6 Compare May 20, 2023 21:23
@hajoha
Copy link

hajoha commented Jun 22, 2023

Looks good to me, thanks for the fix!

@bmah888
Copy link
Contributor

bmah888 commented Jun 26, 2023

Thanks for the PR! I'm starting to look at some iperf3 stuff again (other job priorities), looking forward to trying this out.

@bmah888
Copy link
Contributor

bmah888 commented Jul 3, 2023

Looking good and it definitely fixes the problem from #1405. By the way, I very much appreciate your doing the stylistic changes in 9a7edb4 as a separate commit from the other functional changes.

@bmah888 bmah888 linked an issue Jul 3, 2023 that may be closed by this pull request
@bmah888 bmah888 merged commit cc10897 into esnet:master Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exit Value of iperf while using --json output
3 participants