fix(libcanhub): poll(2) timeout semantics and connect failure detail - #163
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Give every
timeout_msthe samepoll(2)meaning, and makecanhub_last_error(NULL)explain whycanhub_connectfailed instead of crashing.Why
Audit findings API-3 and API-4, both HIGH.
effectiveTimeoutturnedtimeout_ms <= 0into 5000 ms for connect/list/open, so-1meant "block forever" incanhub_recvand "wait 5 s" everywhere else — whiledoc/libcanhub.mdpromised the former and the shipped example passed-1expecting it.0meant "5 s default" in list/open but "non-blocking" in recv.canhub_last_errordereferenced the session with no NULL check.canhub_connectreturns NULL for eight distinct failures and leaves no session to ask, so the naturalcanhub_last_error(NULL)segfaulted on the one failure it most needed to explain.How
-1blocks,0returns immediately,>0is a deadline.effectiveTimeoutis gone and the three loops carry the sametimeout_ms >= 0guardcanhub_recvalready had. A thread-local buffer records why the last connect failed; every failure exit sets it andcanhub_last_error(NULL)returns it. Mirrored in the Windows port, which has two extra exits (no unix scheme, winsock init). The Python backend now appends the detail toCanInitializationError, and the example prints it.Behaviour change: callers passing
0tocanhub_list/canhub_openused to get 5 s and now get a non-blocking call. The doc never promised the old behaviour.Testing
make testgreen (402),pytestgreen (14, one new)libcanhub.robot6/6 in the bench container, two new cases asserting the connect-failure textmake windowscross-build green (_Thread_localcompiles under mingw).so:struct_sizemismatch, NULL config and bad url now each report their own reason;-1still blocking after 12 s (was 5 s);0returns in 0.00 smainsegfaults oncanhub_last_error(NULL)Notes
No ABI change: same symbols, same struct layout.
include/canhub.hstays comment-free — the contract lives indoc/libcanhub.md, as before.