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

PR to close #38 #46

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

PR to close #38 #46

wants to merge 2 commits into from

Conversation

muehlke
Copy link

@muehlke muehlke commented Nov 13, 2024

This PR is aimed at fixing the FC timeout error test case so that it checks this error on both tp implementations (ISOTP_C and ISOTP_SOCK).

Tested locally on a Ubuntu machine (24.04.1 LTS, x86_64) with bazel test //test:all and all tests passed, just testing the fuzz server was skipped.

@driftregion
Copy link
Owner

This is a wonderful proof of concept but the implementation is too complex, mostly due to the existing implementation of env.c that you have successfully worked around.

  • Could the test function void TestISOTPFlowControlFrameTimeout(...) be made simpler by making separate test function for each tp type?
  • Consider using a custom setup function that does not register a server tp rather than conditionally calling ENV_TpFree(srv); in the test. Note that unique setup and teardown functions may be passed to cmocka_unit_test_setup_teardown.
  • Ideally, ENV_RunMillis runs everything registered and there is no need to run a specific transport.

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.

2 participants