Update OpenBSD support to new drm API introduced in 6.9 #216
Conversation
|
This was based on upstream Any clue as to how/when this was changing, and is this PR backwards-compatible "enough"? |
It looks like this changed in 2021 openbsd/xenocara@9d1e1e2 OpenBSD only supports the last two releases, with one release every 6 months that means we don't need to worry (besides, breaking API and ABI across releases is not a huge issue in OpenBSD land). |
|
Latest push includes a few fixes based on feedback from @MarijnS95. I also added a check to handle the actual error case "??" properly. I am not convinced that |
We can't really mark a function as thread-unsafe in rust, so I am afraid this would need a global lock variable. |
OpenBSD uses the same names as Linux and FreeBSD since OpenBSD 6.9, see https://cvsweb.openbsd.org/xenocara/lib/libdrm/xf86drm.h?rev=1.27
required for libc::devname
|
Clarified the |
|
libc with |
| } | ||
|
|
||
| let dev_str = | ||
| unsafe { CStr::from_ptr(buf).to_str() }.expect("Returned device name is not valid utf8"); |
There was a problem hiding this comment.
I think to_str() can be outside of the unsafe block where .expect() already resides.
| #[cfg(target_os = "openbsd")] | ||
| use std::sync::Mutex; | ||
|
|
||
| #[cfg(target_os = "openbsd")] | ||
| static DEVNAME_LOCK: Mutex<()> = Mutex::new(()); |
There was a problem hiding this comment.
Would it make sense to keep this static locally inside the function for now? And/or do we expect other calls to devname()?
Note that this won't help if user code - through other means - calls libc::devname() without knowing about "your / our" lock 😅
There was a problem hiding this comment.
Would it make sense to keep this static locally inside the function for now? And/or do we expect other calls to devname()?
Sure, I can do that
Note that this won't help if user code - through other means - calls libc::devname() without knowing about "your / our" lock 😅
True, but that's how the underlying API works so not much we can do about it imho.
tobhe
changed the title
tobhe
changed the title
This makes OpenBSD work properly on my machine.
WIP because it depends on a libc change in rust-lang/libc#4285 (comment)
My rust experience is rather limited so I'm looking forward to hear how this could be improved.
One potential problem I see is that
devname()is not thread-safe as is because the name buffer is static. Does this need to be copied safely, guarded by a lock or marked unsafe?