8000 storage permissions: clarify naming by bradjc · Pull Request #3554 · tock/tock · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

storage permissions: clarify naming #3554

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 24, 2023
Merged

storage permissions: clarify naming #3554

merged 1 commit into from
Jul 24, 2023

Conversation

bradjc
Copy link
Contributor
@bradjc bradjc commented Jul 21, 2023

Pull Request Overview

Tock has a notion of storage permissions that traces from the TBF header to a type in the kernel crate. The naming changes from module-to-module, and this PR attempts to make it consistent.

  • Remove the "persistent acl" terminology. Replace with "storage permissions". This is more clear that it relates to storage and not something else that is persistent.
  • Remove "access id" and replace with "modify id". I think this is more clear, as access and read are similar, but really having an access id gives the app modify permissions.
  • Make the StoragePermissions object in the kernel consistent. Before it had two write ids, the app's write id and a list of write ids the app could modify. That list is now modify ids.

Testing Strategy

This is only a cosmetic change and so verified by the compiler.

TODO or Help Wanted

n/a

If this is good I will update elf2tab and tockloader to match.

Documentation Updated

  • Updated doc/tbf.md

Formatting

  • Ran make prepush.

@bradjc
storage permissions: clarify naming
758a056 
Tock has a notion of storage permissions that traces from the TBF header
to a type in the kernel crate. The naming changes from module-to-module,
and this commit attempts to make it consistent.

- Remove the "persistent acl" terminology. Replace with "storage
  permissions". This is more clear that it relates to storage and not
  something else that is persistent.
- Remove "access id" and replace with "modify id". I think this is more
  clear, as access and read are similar, but really having an access id
  gives the app modify permissions.
- Make the StoragePermissions object in the kernel consistent. Before it
  had two write ids, the app's write id and a list of write ids the app
  could modify. That list is now modify ids.
@github-actions github-actions bot added kernel tock-libraries This affects libraries supported by the Tock project labels Jul 21, 2023
ppannuto
ppannuto approved these changes Jul 21, 2023
View reviewed changes
Copy link
Member
@ppannuto ppannuto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This falls a bit into the name churn bin, which I'd normally nudge to deferring to a major version bump with other messiness, but I think this is worth an exception as it's a key user-facing aspect of the upcoming tutorial.

@bradjc
Copy link
Co 8000 ntributor Author
bradjc commented Jul 24, 2023

This falls a bit into the name churn bin, which I'd normally nudge to deferring to a major version bump with other messiness, but I think this is worth an exception as it's a key user-facing aspect of the upcoming tutorial.

I disagree, it's quite confusing as the name changes with every layer. We also specifically marked this portion of code as experimental with the exact intention we would change it without a major version because we weren't sure how it would work.

@ppannuto ppannuto added this pull request to the merge queue Jul 24, 2023
Merged via the queue into master with commit 75af40e Jul 24, 2023
@ppannuto ppannuto deleted the storage-permissions-naming branch July 24, 2023 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alistair23 alistair23 alistair23 approved these changes

@brghena brghena brghena approved these changes

@ppannuto ppannuto ppannuto approved these changes

@lschuermann lschuermann lschuermann approved these changes

Assignees
No one assigned
Labels
kernel tock-libraries This affects libraries supported by the Tock project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bradjc @alistair23 @brghena @ppannuto @lschuermann
0