Skip to content

Commit

Permalink
Use c_int on Rust side of DeviceInfo_FFI.device_type so we can proper…
Browse files Browse the repository at this point in the history
…ly bounds check it to prevent undefined behaviour
  • Loading branch information
simon-wh committed Oct 30, 2024
1 parent b61774b commit e852ea0
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 10 deletions.
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
17 changes: 11 additions & 6 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,12 +90,11 @@ impl Drop for DeviceInfo_FFI {

impl DeviceInfo_FFI {
pub fn into_device_info(&self) -> DeviceInfo {
let raw = self.device_type.clone() as i32;
let device_type = DeviceType::from_i32(raw);
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: {}",
raw
self.device_type
);
}

Expand Down

0 comments on commit e852ea0

Please sign in to comment.