Add support for converting from winapi types #336

2018-10-06T15:47:52-05:00

Redrield commented

2018-10-06 15:47:52 -05:00

(Migrated from github.com)

I'm submitting a(n) feature

Description

Add a module guid_support that contains methods on Uuid for converting to and from winapi::shared::guiddef::GUID. Along with this an optional dependency on winapi is added.

Motivation

#333

Tests

Not yet, waiting for a VM (will update once I have run tests)

closes #333

**I'm submitting a(n)** feature # Description Add a module `guid_support` that contains methods on Uuid for converting to and from winapi::shared::guiddef::GUID. Along with this an optional dependency on winapi is added. # Motivation #333 # Tests Not yet, waiting for a VM (will update once I have run tests) # Related Issue(s) closes #333

KodrAus (Migrated from github.com) reviewed

2018-10-06 15:47:52 -05:00

kinggoesgaming (Migrated from github.com) requested changes

2018-10-06 15:55:15 -05:00

src/lib.rs

winapi_support

`winapi_support`

kinggoesgaming (Migrated from github.com) requested changes

2018-10-06 15:57:15 -05:00

Cargo.toml

					
				@ -26,6 +26,10 @@ all-features = true

				[package.metadata.playground]

0.3

`0.3`

src/guid_support.rs

use winapi::shared::guiddef

`use winapi::shared::guiddef`

guid: guiddef::GUID

`guid: guiddef::GUID`

guiddef::GUID

`guiddef::GUID`

kinggoesgaming (Migrated from github.com) requested changes

2018-10-06 16:00:48 -05:00

src/winapi_support.rs

					
				@ -0,0 +1,65 @@

				use prelude::*;

Document assumption that the Uuid is LE

Document assumption that the `Uuid` is LE

kinggoesgaming commented

2018-10-06 16:05:05 -05:00

(Migrated from github.com)

@Redrield thanks for working on this.

Get the assumption documented and it will be ready for merge

@Redrield thanks for working on this. Get the assumption documented and it will be ready for merge

Redrield (Migrated from github.com) reviewed

2018-10-06 16:05:27 -05:00

src/winapi_support.rs

					
				@ -0,0 +1,65 @@

				use prelude::*;

I could have it use whichever endianness the Uuid is. Uuid::to_fields_le is the only method I found, and is actually the outlier, with the other methods being prefixed as_

I could have it use whichever endianness the `Uuid` is. `Uuid::to_fields_le` is the only method I found, and is actually the outlier, with the other methods being prefixed `as_`

Redrield commented

2018-10-06 16:19:30 -05:00

(Migrated from github.com)

Doc comments are changed, I'm still waiting for this VM so that I can add tests and such to verify functionality. Once that's done I'll remove [WIP] so that it can be merged

👍 2

kinggoesgaming (Migrated from github.com) approved these changes

2018-10-06 16:46:42 -05:00

Redrield commented

2018-10-06 17:04:04 -05:00

(Migrated from github.com)

Another thing that I could do, now I think of it is make Uuid::to_guid_le and Uuid::to_guid_be if wanted

Another thing that I could do, now I think of it is make `Uuid::to_guid_le` and `Uuid::to_guid_be` if wanted

KodrAus commented

2018-10-06 17:58:34 -05:00

(Migrated from github.com)

Doc comments are changed, I'm still waiting for this VM so that I can add tests and such to verify functionality.

👍 We'll need some roundtrip tests to make sure Uuids converted into Guids result in the same values, and that Uuid assigns the fields of the Guid in the same way as Windows APIs do.

> Doc comments are changed, I'm still waiting for this VM so that I can add tests and such to verify functionality. :+1: We'll need some roundtrip tests to make sure `Uuid`s converted into `Guid`s result in the same values, and that `Uuid` assigns the fields of the `Guid` in the same way as Windows APIs do.

KodrAus commented

2018-10-06 18:02:10 -05:00

(Migrated from github.com)

Another thing that I could do, now I think of it is make Uuid::to_guid_le and Uuid::to_guid_be if wanted

Hmm, I don't think this would make a lot of sense here, would it? Since the fields of the Guid are primitive integers, they'll just use the platform endianness.

> Another thing that I could do, now I think of it is make Uuid::to_guid_le and Uuid::to_guid_be if wanted Hmm, I don't think this would make a lot of sense here, would it? Since the fields of the `Guid` are primitive integers, they'll just use the platform endianness.

👍 2

Redrield commented

2018-10-06 19:48:23 -05:00

(Migrated from github.com)

Got this code for a test

    #[test]
    fn test_conversion() {
        let uuid = Uuid::from_str("735d359d-4bc4-4e07-8c49-eb3e99a048dc").unwrap();
        let guid = uuid.to_guid();
        assert_eq!(Ok(uuid), Uuid::from_guid(guid));
    }

and it's failing, looks like from an endian mismatch. from_guid uses from_fields, so is that endian dependant?

Got this code for a test ``` #[test] fn test_conversion() { let uuid = Uuid::from_str("735d359d-4bc4-4e07-8c49-eb3e99a048dc").unwrap(); let guid = uuid.to_guid(); assert_eq!(Ok(uuid), Uuid::from_guid(guid)); } ``` and it's failing, looks like from an endian mismatch. from_guid uses from_fields, so is that endian dependant?

KodrAus commented

2018-10-06 20:04:50 -05:00

(Migrated from github.com)

@Redrield I'm thinking the from_fields_le call is the problem here and we probably want to use from_fields instead.

@Redrield I'm thinking the `from_fields_le` call is the problem here and we probably want to use `from_fields` instead.

Redrield commented

2018-10-06 20:06:18 -05:00

(Migrated from github.com)

There is no from_fields_le from what I can see. There's from_bytes_le but that takes a single unified array, not the individual fields

There is no `from_fields_le` from what I can see. There's `from_bytes_le` but that takes a single unified array, not the individual fields

KodrAus commented

2018-10-06 20:10:00 -05:00

(Migrated from github.com)

Sorry, I meant to_fields rather than to_fields_le :)

Sorry, I meant `to_fields` rather than `to_fields_le` :)

Redrield commented

2018-10-06 20:13:02 -05:00

(Migrated from github.com)

That was indeed the problem. Are there any other tests that are going to be wanted other than that one? I'll add them before I push the final code for merge

Dylan-DPC-zz commented

2018-10-07 07:29:36 -05:00

(Migrated from github.com)

@Redrield one from guid to uuid will be helpful

Dylan-DPC-zz (Migrated from github.com) approved these changes

2018-10-07 10:05:19 -05:00

Dylan-DPC-zz commented

2018-10-07 10:05:36 -05:00

(Migrated from github.com)

bors: r+

bors[bot] commented

2018-10-07 10:14:39 -05:00

(Migrated from github.com)

Build succeeded

# Build succeeded * [continuous-integration/travis-ci/push](https://travis-ci.org/uuid-rs/uuid/builds/438286184?utm_source=github_status&utm_medium=notification) * [continuous-integration/appveyor/branch](https://ci.appveyor.com/project/KodrAus/uuid/builds/19322517)

Sign in to join this conversation.

No reviewers

KodrAus

kinggoesgaming

Dylan-DPC-zz