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

Validate that resources belong to the right device. #4207

Merged
merged 2 commits into from
Oct 4, 2023

Conversation

nical
Copy link
Contributor

@nical nical commented Oct 4, 2023

Checklist

  • Run cargo clippy.
  • Add change to CHANGELOG.md. See simple instructions inside file.

Connections

Fixes #3927

Description

A pretty boring and mechanical change set although fairly important for the browser use case. Each entry point that takes resources as input must check that these resources are associated with the same device.

I'm sure that I have forgotten some, thankfully it looks like the CTS is rather thorough about testing this so we'll see what's missing when this makes its way into firefox.

Testing

I enabled the the device_msimatch test which I added when filing the issue. It only covers a tiny portion of the cases but the CTS has good coverage of this so adding exhaustive tests here is not as important.

@nical nical requested a review from a team as a code owner October 4, 2023 13:42
@nical nical requested a review from a team October 4, 2023 13:46
Copy link
Member

@ErichDonGubler ErichDonGubler left a comment

Choose a reason for hiding this comment

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

These changes LGTM. I'm paranoid and want to make sure we cover all of the API that's necessary, but I guess that need not block. We can keep iterating and covering more cases with follow-up work.

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

In the future, I think it would be nice to have a trait like

trait DeviceChild {
    fn get_device_id(&self) -> ....;
    
    fn is_child_of(&self, device: Id<Device>) -> Result<(), DeviceError> {
        ....
    }
}

or something to streamline this a bit and prevent mistakes, but for now this is fine.

@cwfitzgerald cwfitzgerald merged commit 198e1df into gfx-rs:trunk Oct 4, 2023
23 checks passed
@nical nical deleted the same-device branch October 4, 2023 18:52
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.

Validate that inputs are created from the right device
3 participants