Move libfsm/start.c to Rust #344
Open
Conversation
This file contains 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
This is a shame, but gets in the way of porting to Rust.
… the partially linked .o files. This hopefully avoids an ld bug where `-undefined dynamic_lookup` doesn't work on MacOS, and also removes the need for programs to link both the C and Rust libraries directly.
C bitfields in Rust are painful, and I'd like something that has the same ABI as Rust's bool. stdbool.h's bool is just the thing. In both struct fsm_state and struct fsm: * All the bitfields are packed together. * The field that follows the bitfields is a pointer, so it has pointer alignment, so the overall size of the structs shouldn't change.
These come from internal.h. For now the Rust types have the same ABI as the C types; we'll have parallel structs for a while until all the internals get rustified.
The strategy for this commit: * Implement clear_start / set_start / get_start in Rust as methods, with argument checks as assertions. * Implement a C ABI wrapper to match the fsm_clearstart() / etc. functions. The non-null argument checks go there. * Test both sets of functions right there. We cannot construct a Fsm in Rust just yet, so the tests hack up just enough initial state for the code to run; this can be removed later. * Remove start.c. Later we can move all of the C API to a c_api.rs or whatever.
I really don't know the best way to do this with pmake; someone who knows better should fix this :)
Another easy function, wheeee!
Instead of duplicating the code.
…loc block In all places where a state's .has_capture_actions gets accessed, there is an assert that the index of the state fits within the fsm->statecount. Since in the previous commit we initialize the has_capture_actions field when a new state is added, there is no need to initialize that field for all the as-yet-unused state structs in the realloc block.
The -1 was meant to be an OOM sentinel, but there is no place in the code that actually sets fsm->statecount to -1 when the states array cannot be (re)allocated. The code after the patch does the realloc(), and properly returns a failure code if realloc() fails. I don't think we need to preserve setting "errno = ENOMEM" since there is no other place in the code that signals OOM through errno.
…whole statealloc Only the first statecount state structs are valid, anyway.
|
The last commits I've pushed ( Those commits are useful outside of a Rust port. Please tell me if you'd rather have me submit a separate pull request for them, or if you'd prefer to cherry-pick them or whatever. |
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.
This depends on #343.
First we replace the bitfields in a couple of structs with
stdbool.h, so we can have the same ABI as Rust'sbooltype.Then we make parallel structs in Rust for
struct fsm_stateandstruct fsm. We'll keep these in sync with the C structs while the internals get rustified.Then we port
fsm_clearstart,fsm_getstart,fsm_setstartto Rust, and removestart.c.I probably totally botched the Makefiles to add
-lpthread -ldl, needed by Rust libraries, but I need some help from someone who knows pmake :)