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

Add a version of Uuid::from_fields that can't fail #150

Closed
luser opened this issue Feb 15, 2018 · 12 comments · Fixed by #536
Closed

Add a version of Uuid::from_fields that can't fail #150

luser opened this issue Feb 15, 2018 · 12 comments · Fixed by #536

Comments

@luser
Copy link

luser commented Feb 15, 2018

Uuid::from_fields takes an &[u8] for d4, so it will return an error if d4 is not 8 bytes in length. Every time I've used this method I've had an 8 byte array to hand, so it seems silly to have to call unwrap on the result. It would be nice if there was a version of this like fn from_fields(d1: u32, d2: u16, d3: u16, d4: &[u8; 8]) -> Uuid.

I realize that working with arrays is sort of a pain in Rust for several reasons. Once the const generics RFC gets implemented it should make some things simpler, like making it possible to have a method on slices that returns an array reference of a certain size.

@Dylan-DPC-zz
Copy link
Member

Hi Ted,

Thanks. We will look into this. Instead of having seperate functions, I would prefer if we could use generics for this, if possible.

The earliest we can get this implemented & released is either 0.6.1 or 0.7.0.

Also you are free to send us a PR implementing this.

@Dylan-DPC-zz Dylan-DPC-zz added this to the Post 0.6.0 milestone Feb 15, 2018
@luser
Copy link
Author

luser commented Feb 15, 2018

I'm not sure what precisely you'd want to do with generics here, can you clarify?

@Dylan-DPC-zz
Copy link
Member

I was thinking if we could have just one function that returns the UUID irrespective of data. But I think that won't be possible. Will add the function as you proposed.

@KodrAus
Copy link
Member

KodrAus commented Feb 15, 2018

It's not quite the same as the proposed from_fields method (since it takes the complete 16 byte array) but #145 offers an infallible way to get a Uuid from its raw representation.

@luser
Copy link
Author

luser commented Feb 16, 2018

Oh, that is indeed very similar. In the uses I've found I can't just pass [u8; 16] because I'm reading the data from a file and it might need to be byte-swapped, unfortunately.

@kinggoesgaming
Copy link
Member

I'm reading the data from a file and it might need to be byte-swapped, unfortunately.

I think that's a niche case, that should technically be handled by the library user before passing it to the library.

@luser
Copy link
Author

luser commented Feb 20, 2018

I think that's a niche case, that should technically be handled by the library user before passing it to the library.

I am doing so, I just then don't have a method on Uuid I can call without needing to call unwrap on the result.

@kinggoesgaming
Copy link
Member

@luser Can I see an example of how you are using this? Perhaps it might help get a better understanding on this

@luser
Copy link
Author

luser commented Feb 20, 2018

Sure. This code hasn't actually landed anywhere yet, but it's a proposed patch to goblin:
https://github.com/luser/goblin/blob/b541e9584322c7e9c8e04b89db6d9be14d77e5c0/src/pe/debug.rs#L106

@Dylan-DPC-zz
Copy link
Member

Looking again at this, i feel that having a similar method with the only change being a different return type doesn't make much sense to me. It will add more cost ot the maintainence later and there isn't much gain including it. We could provide a function to unwrap but then it will be the same as calling unwrap from outside and there is no visible gain in it.

@luser
Copy link
Author

luser commented Feb 22, 2018

You are probably right. I think in the future when we have const generics and it's possible to easily get a reference to an array of a specific size from a slice designing APIs that use exactly-sized array references will be more ergonomic.

@kinggoesgaming kinggoesgaming modified the milestones: 0.6.1, 0.7.0 Mar 1, 2018
@Dylan-DPC-zz Dylan-DPC-zz modified the milestones: 0.7.0, 0.6.2 Mar 1, 2018
@Dylan-DPC-zz Dylan-DPC-zz removed this from the 0.6.2 milestone Mar 19, 2018
@KodrAus
Copy link
Member

KodrAus commented Oct 28, 2021

I’ve ended up circling back around to this while making a lot more things const. Fallible methods are still tricky for const.

The approach I was thinking of was to make from_fields take a &[u8; 8] and add a fallible from_fields_slice that takes a &[u8] for convenience. You can also still use try_into to convert a &[u8] into a &[u8; 8] now.

That would make Uuid::from_fields much more useful in const contexts.

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 a pull request may close this issue.

4 participants