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

std::mem::replace doesn't work on Sid #4

Open
roblabla opened this issue May 24, 2021 · 2 comments
Open

std::mem::replace doesn't work on Sid #4

roblabla opened this issue May 24, 2021 · 2 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@roblabla
Copy link
Contributor

roblabla commented May 24, 2021

Because Sid is an opaque type, it is impossible to std::mem::replace it. Example test:

    /// Ensures that std::mem::replace works properly with Sid
    #[test]
    fn mem_replace() {
        let mut sid1 = crate::Sid::well_known_sid(winapi::um::winnt::WinWorldSid).unwrap();
        let mut sid2 = crate::Sid::well_known_sid(winapi::um::winnt::WinNtAuthoritySid).unwrap();
        assert_eq!(sid1.id_authority(), &[0, 0, 0, 0, 0, 1]);
        assert_eq!(sid2.id_authority(), &[0, 0, 0, 0, 0, 5]);

        std::mem::swap(&mut *sid1, &mut *sid2);

        assert_eq!(sid1.id_authority(), &[0, 0, 0, 0, 0, 5]);
        assert_eq!(sid2.id_authority(), &[0, 0, 0, 0, 0, 1]);
    }

Fixing this is, erm, complicated. I suppose the ideal solution would be to make Sid an unsized type ([u8]) instead of an opaque type. This would cause mem::replace to become a compile time error, I believe, but would turn pointers to Sid into fat pointers.

EDIT: I think what we'd need is rust-lang/rust#43467. If Sid was an Opaque Type, it'd be !Sized, and thus mem::replace wouldn't work, but a reference to it would still be only a single pointer.

@danieldulaney
Copy link
Owner

I think that's correct. I don't know of any way to explicitly mark a type as !Sized, but the goal is that it should only be accessible through a pointer.

For what it's worth, swapping does work with std::mem::swap(&mut sid1, &mut sid2). This is definitely a bug until there is a way to mark a struct !Sized.

@danieldulaney
Copy link
Owner

Also, the example uses mem_swap and I think that's the main issue. mem_replace should be fine because you need a raw Sid (not behind a pointer) to call it, and there should never be a way to get hold of one of those.

@danieldulaney danieldulaney added bug Something isn't working help wanted Extra attention is needed labels May 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants