Port to windows-sys
#494
Port to windows-sys
#494
Conversation
|
I would somewhat prefer we keep these bindings the way they are, given that this is included in the stdlib. It seems somewhat risky to switch to windows-sys, which has a fairly idiosyncratic way of linking (not saying it's bad, but I'm not sure it's something we want to do for effectively all rust users...) |
|
Going to close this for now - will be back! 😉 |
I'd love to understand what you mean by this. What's idiosyncratic about the way it links? |
|
Yeah, it's definitely possible I'm being a bit paranoid and it would be fine. I would like us to be sure that that's the case before we merge this though, and do it with the understanding that this cause those to be linked into every Rust program. For example, in the past I had issues in cases where symbols were provided both by |
|
Btw, incorporating windows-sys into stdlib proper is very much on my radar. I think the implementation of the raw-dylib feature would be a good point to start doing this. Maintaining separate bindings is error prone and, imho, wasted effort so it'd be great if there's only one source of truth. |
|
Thanks for the feedback! I was just chatting with @wesleywiser and he suggested I take a stab at porting I will say the libs have come a long way since those early days. The libs are now quite mature and specifically avoid overlaps with non-Windows APIs like SQLite. I would say they're now also more robust and correct than the libs that you get via |
|
I'd love to have all these manual bindings deleted at some point as well, it's of course good to centralize these sorts of definitions in a vetted location like That being said the hardest part of this transition is probably going to be how |
|
Sounds good - let me know if I can help in any way. |
There was a problem hiding this comment.
The type aliases in the Windows bindings are effectively meaningless because DWORD, etc. are "locked" to their current definitions even on CPUs with different word sizes due to architectural inertia, so I am fine with seeing those aliases go. However, some C libraries are...quirky enough to adopt idioms that amount to
const TRUE: c_int = 0;
const FALSE: c_int = -1;or similar. ( I'm looking at you, Unix! ) So I would prefer slightly-more-meaningful constants remain wherever they are currently present, and I intend to add more such value names to the existing modules in this crate so it can better serve as what it already is: a Rosetta Stone of backtrace and symbolication libraries.
| @@ -162,7 +167,7 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { | |||
| Some(get_module_base), | |||
| None, | |||
| 0, | |||
| ) == TRUE | |||
| ) == 1 | |||
There was a problem hiding this comment.
I would prefer to still see a name for meaningful "magic numbers" like this one, simply because I am going to ask everyone else to also name magic numbers and sentinel values for their OS.
| @@ -93,13 +70,13 @@ macro_rules! dbghelp { | |||
| /// | |||
| /// Panics if library is already loaded. | |||
| fn ensure_open(&mut self) -> Result<(), ()> { | |||
| if !self.dll.is_null() { | |||
| if self.dll != 0 { | |||
There was a problem hiding this comment.
Remarks about preferring a name for sentinel values apply to this file as well. In this case it's a novel concern due to a pointer now being an integer so some comparisons are "simplified".
Oops, didn't mean to submit this just yet. 😊
Anyway, this is a simple port of the
backtracecrate fromwinapitowindows-sys. It ensures thatdbghelp.dllis still delay loaded, in the same way as before, while avoiding a ton of unnecessary redefinitions.