Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Safer handling of DeviceType value coming from FFI #90

Merged
merged 2 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions includes/wooting-analog-common.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ typedef struct WootingAnalog_DeviceInfo_FFI {
*/
WootingAnalog_DeviceID device_id;
/**
* Hardware type of the Device
* Hardware type of the Device see `DeviceType` enum
*/
enum WootingAnalog_DeviceType device_type;
WootingAnalog_DeviceType device_type;
} WootingAnalog_DeviceInfo_FFI;
12 changes: 10 additions & 2 deletions wooting-analog-common/cbindgen.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,25 @@ header = "/* This is a generated header file providing the common items to every
autogen_warning = "/* Warning, this file is autogenerated by cbindgen. Don't modify this manually. */"

[export]
include = ["DeviceInfoBlank", "DeviceInfo_FFI", "DeviceEventType", "WootingAnalogResult", "KeycodeType"]
include = [
"DeviceInfoBlank",
"DeviceInfo_FFI",
"DeviceEventType",
"WootingAnalogResult",
"KeycodeType",
"DeviceType",
]
prefix = "WootingAnalog_"
renaming_overrides_prefixing = true
item_types = ["enums", "structs", "typedefs", "functions", "opaque"]

[export.rename]
"FfiStr" = "const char*"
"WootingAnalogResult" = "WootingAnalogResult"
"DeviceType_FFI" = "WootingAnalog_DeviceType"

[parse]
expand = ["wooting-analog-common"]

[enum]
prefix_with_name=true
prefix_with_name = true
22 changes: 18 additions & 4 deletions wooting-analog-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ pub struct DeviceInfo {
pub device_type: DeviceType,
}

// We do this little alias so that we can force cbindgen to rename it to point to the DeviceType enum.
// For the rust side, we want to have it as a c_int so we can ensure it's valid and within bounds.
// As just taking it as the enum straight up can cause undefined behaviour if an invalid value is provided
/// cbindgen:ignore
type DeviceType_FFI = c_int;

/// The core `DeviceInfo` struct which contains all the interesting information
/// for a particular device. This is the version which the consumer of the SDK will receive
/// through the wrapper. This is not for use in the Internal workings of the SDK, that is what
Expand All @@ -55,8 +61,8 @@ pub struct DeviceInfo_FFI {
pub device_name: *mut c_char,
/// Unique device ID, which should be generated using `generate_device_id`
pub device_id: DeviceID,
/// Hardware type of the Device
pub device_type: DeviceType,
/// Hardware type of the Device see `DeviceType` enum
pub device_type: DeviceType_FFI,
}

impl From<DeviceInfo> for DeviceInfo_FFI {
Expand All @@ -67,7 +73,7 @@ impl From<DeviceInfo> for DeviceInfo_FFI {
manufacturer_name: CString::new(device.manufacturer_name).unwrap().into_raw(),
device_name: CString::new(device.device_name).unwrap().into_raw(),
device_id: device.device_id,
device_type: device.device_type,
device_type: device.device_type as c_int,
}
}
}
Expand All @@ -84,6 +90,14 @@ impl Drop for DeviceInfo_FFI {

impl DeviceInfo_FFI {
pub fn into_device_info(&self) -> DeviceInfo {
let device_type = DeviceType::from_i32(self.device_type);
if device_type.is_none() {
log::error!(
"Invalid Device Type when converting DeviceInfo_FFI into DeviceInfo: {}",
self.device_type
);
}

DeviceInfo {
vendor_id: self.vendor_id.clone(),
product_id: self.product_id.clone(),
Expand All @@ -102,7 +116,7 @@ impl DeviceInfo_FFI {
.to_owned()
},
device_id: self.device_id.clone(),
device_type: self.device_type.clone(),
device_type: device_type.unwrap_or(DeviceType::Other),
}
}
}
Expand Down