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

Ensure that furi_record_create is passed a non-NULL data pointer #4078

Merged
merged 3 commits into from
Feb 11, 2025

Conversation

dcoles
Copy link
Contributor

@dcoles dcoles commented Jan 24, 2025

It's currently possible to use furi_record_create to create and initialize a FuriRecordData pointing to NULL.

This means its potentially possible for furi_record_open to return a NULL pointer which besides not being particularly useful means the Rust wrapper for Record can't assume that the returned record is always a non-NULL value.

If by chance this is the intended behaviour, then we can just have the Rust wrapper do a furi_check itself, but it seems like it would be better to eliminate this potential corner-case at the source.

What's new

  • furi_record_create now checks that data is a non-NULL pointer

Verification

  • Calling furi_record_create with a NULL data pointer should trigger a furi_check system crash

Checklist (For Reviewer)

  • PR has description of feature/bug or link to Confluence/Jira task
  • Description contains actions to verify feature/bugfix
  • I've built this code, uploaded it to the device and verified feature/bugfix

It's currently possible to use `furi_record_create` to create and initialize a `FuriRecordData` pointing to NULL.

This means its potentially possible for `furi_record_open` to return a NULL pointer which besides not being particularly useful means the Rust wrapper for `Record` can't assume that the returned record is always a non-NULL value.

If by chance this is the intended behaviour, then we can just have the Rust wrapper do a `furi_check` itself, but it seems like it would be better to eliminate this potential corner-case at the source.
skotopes
skotopes previously approved these changes Feb 11, 2025
@skotopes
Copy link
Member

To be honest there were an option to pass NULL and use record as simple synchronization primitive, but @hedger changed this contract by explicitly marking furi_record_open return value as non-null.

I'll accept this PR and update docs

@skotopes skotopes merged commit 8059959 into flipperdevices:dev Feb 11, 2025
10 of 11 checks passed
@dcoles dcoles deleted the dcoles-patch-1 branch February 12, 2025 00:18
@dcoles
Copy link
Contributor Author

dcoles commented Feb 12, 2025

Thanks for the background. It's nice to understand why it works that way.
Records are one of my favourite parts about working with the Furi API.

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