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 task_handle type mismatch on task spawn #53

Merged
merged 1 commit into from
Mar 22, 2023

Conversation

agerasev
Copy link
Contributor

freertos_rs_spawn_task from shim.c (as well as xTaskCreate) takes TaskHandle_t * as last argument.

But in shim.rs this function takes FreeRtosMutTaskHandle (*mut CVoid) which is not the same as *mut FreeRtosTaskHandle (*mut *const CVoid).

So xTaskCreate tries to store TaskHandle_t into CVoid, but they may have different size and alignment.

This PR fixes this issue.

@agerasev
Copy link
Contributor Author

And also a question: why not to use core::ffi::c_void rather than create custom CVoid?

freertos-rust/src/task.rs Outdated Show resolved Hide resolved
@schteve
Copy link
Collaborator

schteve commented Mar 21, 2023

Good catch, it seems like this only worked so far because likely the CVoid size matches the pointer size in many cases.

As to the question of why CVoid, I don't have any answer. It looks like the implementation of core::ffi::c_void is similar to what was done here with two-variant enum. Maybe there is an opportunity to use the standard type.

@agerasev agerasev force-pushed the feature-fix-spawned-task-handle branch from 8f615a6 to d2aa596 Compare March 21, 2023 03:28
@agerasev
Copy link
Contributor Author

agerasev commented Mar 21, 2023

As to the question of why CVoid, I don't have any answer. It looks like the implementation of core::ffi::c_void is similar to what was done here with two-variant enum. Maybe there is an opportunity to use the standard type.

Ok, thanks, I'll try to switch to core::ffi::c_void and open a separate PR.

@agerasev
Copy link
Contributor Author

agerasev commented Mar 21, 2023

Ok, thanks, I'll try to switch to core::ffi::c_void and open a separate PR.

Opened: #55

@schteve
Copy link
Collaborator

schteve commented Mar 22, 2023

Thanks!

@schteve schteve merged commit 657a844 into lobaro:master Mar 22, 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.

None yet

2 participants