Load the symbols dynamically - #1
Conversation
|
Note: I just got this to build and I haven't yet tested it on a device, I'm putting it up for review early so that you can take the time to look at the API etc, don't hesitate to give the logic itself a thorough look and if you are OK with the new API, don't merge it before you or I have had a chance to properly test it. Also, for reference here is how Firefox does something similar for ICU: https://searchfox.org/mozilla-central/rev/bf6f194694c9d1ae92847f3d4e4c15c2486f3200/intl/icu/source/common/putil.cpp#1082 |
Wgpu needs to load android property symbols dynamically in order to allow a single binary to ship to new and old versions of Android, which Firefox requires. In addition, short of using jni, this is the recommended way. This patch represents a larger change than I would have liked, but it tries to trike a balance between enabling dynamic loading and the previous API. The main change is that an AndroidProperties object is created, containing the dlopened libc as well as the required function pointers. The functions become methods of this object.
|
|
||
| // Fall back to the older approach. | ||
| if let Some(get_fn) = self.get_fn { | ||
| const PROPERTY_VALUE_MAX: usize = 92; |
There was a problem hiding this comment.
It'd be nice to have a comment explaining the origin of this number.
| let raw = cvalue.into_raw(); | ||
| let ret = unsafe { (get_fn)(cname, raw) }; | ||
| match ret { | ||
| len if len > 0 => unsafe { Some(String::from_raw_parts(raw as *mut u8, len as usize, PROPERTY_VALUE_MAX)) }, |
There was a problem hiding this comment.
Does Android guarantee that get_fn returns UTF-8? Constructing a String from bad UTF-8 is UB. I assume that's why property_callback calls CStr's to_str method, which checks for valid UTF-8.
| Err(format!("Failed to set Android property \"{}\" to \"{}\" (error code: {})", name, value, ret)) | ||
| } | ||
| } else { | ||
| Err(format!("Unable to set any Android preperty")) |
2 participants
Wgpu needs to load android property symbols dynamically in order to allow a single binary to ship to new and old versions of Android, which Firefox requires. In addition, short of using jni, this is the recommended way.
This patch represents a larger change than I would have liked, but it tries to trike a balance between enabling dynamic loading and staying true to the previous API. The main change is that an AndroidProperties object is created, containing the dlopened libc as well as the required function pointers. The functions become methods of this object.
Apologies for the large diff, here are a few upsides which hopefully make up for it: