Cleanup

[jesdazrez]

• Bumps widestring to v0.4
• 2021 edition
• Clippy fixes
• winapi -> windows
• Adds enum example
• Fixes some potential UB

[arcnmx]

Neat, thanks for the PR! I'll take a closer look at this soon, but do have some initial comments:

2021 edition

Is there anything explicitly 2021 here or would 2018 suffice? I'm not strictly opposed to 2021, but it might be worth relaxing the MSRV if there's no need to.

Bumps widestring to v0.4

Why not 0.5 (and from what I can tell, a 1.0 is about to be released)?

winapi -> windows

I haven't been keeping up with the ecosystem, what is the rationale here? this issue in particular caught my eye, and is a dealbreaker if still true.

Fixes some potential UB

I was confused by this at first, but I'm guessing it has something to do with appeasing a rustc warning around packed struct accesses? I highly doubt there's UB or unaligned accesses happening there unless the whole container struct is unaligned - and if it is, this change seems unlikely to actually fix it (if widestring's from_ptr_str assumes that the provided pointer is aligned, which is likely). It just feels like the commit wording here may be misleading?

chore: v0.3.1

With winapi part of the public api that makes this a breaking change to 0.4.0

---

It might be worth splitting this into multiple PRs to make it easier to discuss/review/land?

[jesdazrez]

Hi there, sorry about closing this with no response. I think I inadvertently closed it when merging this in https://github.com/AstroHQ/ddc-winapi-rs.

Is there anything explicitly 2021 here or would 2018 suffice? I'm not strictly opposed to 2021, but it might be worth relaxing the MSRV if there's no need to.

2018 is enough.

Why not 0.5 (and from what I can tell, a 1.0 is about to be released)?

I will updated it to latest in a new PR.

I haven't been keeping up with the ecosystem, what is the rationale here? this issue in particular caught my eye, and is a dealbreaker if still true.

I haven't tried cross-compiling from Linux. Mostly the switch was motivated by our good experience with the better ergonomics of the windows-rs bindings (IntoParam trait) that makes it feel a lot more "rusty". I can exclude this change when making separate PR for the deps update.

I was confused by this at first, but I'm guessing it has something to do with appeasing a rustc warning around packed struct accesses? I highly doubt there's UB or unaligned accesses happening there unless the whole container struct is unaligned - and if it is, this change seems unlikely to actually fix it (if widestring's from_ptr_str assumes that the provided pointer is aligned, which is likely). It just feels like the commit wording here may be misleading?

AFAIK just creating a reference to a field of a packed struct is UB (https://doc.rust-lang.org/nightly/std/ptr/fn.read_unaligned.html#on-packed-structs). However you make a good point about the widestring fn also requiring the ptr to have proper alignment. I can make a new PR where I copy the data and pass that copy to widestring. I guess that would fix all of it?

Thanks for the reply!, I'll open new shorter PRs probably the next week :)

[jesdazrez]

btw, I just noticed recently I opened a new one with basically the same content. Sorry for the mess, I'll close that one too 😅.

[arcnmx]

No problem! I saw the new PR but hadn't had a chance to go over it yet. I plan on pulling in most of these changes anyway, it was just a bit hard to review all at once.

I haven't tried cross-compiling from Linux

I recently set up cross compiles as part of ddcset's CI, but haven't tested it with the windows-rs change. If you fork it with a dependency override in Cargo.toml, we can see if it still builds or not.
Otherwise an alternative might be to use a feature to conditionally select the backend winapi crate (though this is tricky for a number of reasons).

AFAIK just creating a reference to a field of a packed struct is UB

It's a bit less absolute IIRC - it's only UB if the reference created is actually unaligned for the referenced type. And since WideString later dereferences it, the potential UB wasn't "fixed" other than hiding the warning.

However you make a good point about the widestring fn also requiring the ptr to have proper alignment. I can make a new PR where I copy the data and pass that copy to widestring.

A proper fix would actually be to just ensure that it's aligned. I feel like the fact that the structure is marked "packed" may be a bug to begin with (or maybe that's just windows.h convention?), but... it's easy enough to just make Monitor an aligned type. The only tricky part is that we'd also want the vec initializer to be a Vec<Monitor> so that it's aligned, too.