From 98f64fdecadddabd3092109eea6a481cff4a1f00 Mon Sep 17 00:00:00 2001 From: Lahfir Date: Thu, 16 Apr 2026 04:20:55 -0700 Subject: [PATCH] feat(ffi): opaque list handles + image buffer length encapsulation (Unit 5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces every `(*mut T, count)` list-returning API with an opaque handle and encapsulates AdImageBuffer's byte-buffer length. Closes R8 and R22 from PR #22 review. ## Opaque list handles Four new one-type-per-file opaque wrappers (no `#[repr(C)]` — cbindgen auto-emits as `typedef struct AdFoo AdFoo;` forward declarations): - crates/ffi/src/types/window_list.rs — AdWindowList - crates/ffi/src/types/app_list.rs — AdAppList - crates/ffi/src/types/surface_list.rs — AdSurfaceList - crates/ffi/src/types/notification_list.rs — AdNotificationList (used by Unit 8) Each list owns its `Box<[AdXxxInfo]>`. Consumers walk through `_count(list)`, `_get(list, index) -> *const AdXxxInfo` (null on OOB), and free with `_free(list)` — the free walks the entries, releases their interior C-strings, and drops the Box. Rewritten signatures: | Old | New | |------------------------------------------------------------|-----------------------------------------------------------| | ad_list_apps(adapter, \*\*apps, \*count) | ad_list_apps(adapter, \*\*list) | | ad_list_windows(adapter, filter, focused, \*\*wins, \*count)| ad_list_windows(adapter, filter, focused, \*\*list) | | ad_list_surfaces(adapter, pid, \*\*sfs, \*count) | ad_list_surfaces(adapter, pid, \*\*list) | | ad_free_apps(apps, count) | ad_app_list_free(list) | | ad_free_windows(wins, count) | ad_window_list_free(list) | | ad_free_surfaces(sfs, count) | ad_surface_list_free(list) | | ad_free_window(win) [for single AdWindowInfo] | ad_release_window_fields(win) | Count mismatches are impossible by construction — callers never see the backing pointer or length. ## Image buffer encapsulation crates/ffi/src/types/image_buffer.rs: dropped `#[repr(C)]`, private `Box<[u8]>` data field, private width/height/format. Before, a C caller who mutated `AdImageBuffer.data_len` triggered heap corruption at free time; now the length is authoritative inside the Rust-owned struct. New accessors in crates/ffi/src/screenshot/accessors.rs: - ad_image_buffer_data -> *const u8 - ad_image_buffer_size -> u64 (always matches the allocation) - ad_image_buffer_width -> u32 - ad_image_buffer_height -> u32 - ad_image_buffer_format -> AdImageFormat ad_screenshot signature changed: `*mut *mut AdImageBuffer out` instead of `*mut AdImageBuffer out`. ad_free_image renamed to ad_image_buffer_free for consistency with the list-handle pattern. All new files stay under 120 LOC, explicit `pub use` per type, no wildcard imports, no inline `//` comments. `///` docs on every public FFI export cover null-tolerance, lifetime, and safety requirements. 53 tests pass. Clippy clean. --- crates/ffi/include/agent_desktop.h | 202 +++++++++++++++++--- crates/ffi/src/apps/list.rs | 75 +++++--- crates/ffi/src/lib.rs | 7 + crates/ffi/src/screenshot/accessors.rs | 70 +++++++ crates/ffi/src/screenshot/capture.rs | 31 +-- crates/ffi/src/screenshot/free.rs | 19 +- crates/ffi/src/screenshot/mod.rs | 1 + crates/ffi/src/surfaces/list.rs | 74 ++++--- crates/ffi/src/types/app_list.rs | 7 + crates/ffi/src/types/find_query.rs | 8 + crates/ffi/src/types/image_buffer.rs | 15 +- crates/ffi/src/types/mod.rs | 14 ++ crates/ffi/src/types/notification_filter.rs | 9 + crates/ffi/src/types/notification_info.rs | 11 ++ crates/ffi/src/types/notification_list.rs | 6 + crates/ffi/src/types/surface_list.rs | 7 + crates/ffi/src/types/window_list.rs | 13 ++ crates/ffi/src/windows/free_one.rs | 16 +- crates/ffi/src/windows/list.rs | 72 ++++--- 19 files changed, 518 insertions(+), 139 deletions(-) create mode 100644 crates/ffi/src/screenshot/accessors.rs create mode 100644 crates/ffi/src/types/app_list.rs create mode 100644 crates/ffi/src/types/find_query.rs create mode 100644 crates/ffi/src/types/notification_filter.rs create mode 100644 crates/ffi/src/types/notification_info.rs create mode 100644 crates/ffi/src/types/notification_list.rs create mode 100644 crates/ffi/src/types/surface_list.rs create mode 100644 crates/ffi/src/types/window_list.rs diff --git a/crates/ffi/include/agent_desktop.h b/crates/ffi/include/agent_desktop.h index aefb6dd..24e7abc 100644 --- a/crates/ffi/include/agent_desktop.h +++ b/crates/ffi/include/agent_desktop.h @@ -113,6 +113,39 @@ typedef int32_t AdWindowOpKind; typedef struct AdAdapter AdAdapter; +/** + * Opaque list handle emitted by `ad_list_apps`. See + * [`crate::types::window_list::AdWindowList`] for the pattern. + */ +typedef struct AdAppList AdAppList; + +/** + * Opaque image-buffer handle returned by `ad_screenshot`. The backing + * byte buffer and its length live inside the Rust-owned struct — a + * consumer cannot accidentally desynchronize the pair and trigger a + * heap-corruption double-free. Walk it through `ad_image_buffer_*` + * accessors and free it with `ad_image_buffer_free`. + */ +typedef struct AdImageBuffer AdImageBuffer; + +/** + * Opaque list handle emitted by `ad_list_surfaces`. See + * [`crate::types::window_list::AdWindowList`] for the pattern. + */ +typedef struct AdSurfaceList AdSurfaceList; + +/** + * Opaque list handle emitted by `ad_list_windows`. + * + * The struct intentionally has no `#[repr(C)]` so cbindgen emits a + * forward declaration only (`typedef struct AdWindowList AdWindowList;`). + * Consumers cannot read the backing pointer or length and cannot + * construct a count mismatch — they walk the list through + * `ad_window_list_count`, `ad_window_list_get`, and free it with + * `ad_window_list_free`. + */ +typedef struct AdWindowList AdWindowList; + typedef struct AdNativeHandle { const void *ptr; } AdNativeHandle; @@ -204,14 +237,6 @@ typedef struct AdScreenshotTarget { int32_t pid; } AdScreenshotTarget; -typedef struct AdImageBuffer { - const uint8_t *data; - uint64_t data_len; - AdImageFormat format; - uint32_t width; - uint32_t height; -} AdImageBuffer; - typedef struct AdSurfaceInfo { const char *kind; const char *title; @@ -341,15 +366,35 @@ AdResult ad_launch_app(const struct AdAdapter *adapter, /** * # Safety * `adapter` must be a valid pointer from `ad_adapter_create`. - * `out` and `out_count` must be valid writable pointers. + * `out` must be a valid writable `*mut *mut AdAppList`. + * On success, `*out` is a newly-allocated opaque list freed with + * `ad_app_list_free`. On error, `*out` is null and last-error is set. */ -AdResult ad_list_apps(const struct AdAdapter *adapter, struct AdAppInfo **out, uint32_t *out_count); +AdResult ad_list_apps(const struct AdAdapter *adapter, struct AdAppList **out); /** * # Safety - * `apps` must be null or a pointer previously returned by `ad_list_apps`. + * `list` must be null or a pointer returned by `ad_list_apps`. */ -void ad_free_apps(struct AdAppInfo *apps, uint32_t count); +uint32_t ad_app_list_count(const struct AdAppList *list); + +/** + * Returns a borrowed pointer into the list; valid until the list is freed. + * Out-of-range `index` returns null. + * + * # Safety + * `list` must be null or a pointer returned by `ad_list_apps`. + */ +const struct AdAppInfo *ad_app_list_get(const struct AdAppList *list, uint32_t index); + +/** + * Frees the list and every `AdAppInfo` it owns, including the interior + * C-strings. + * + * # Safety + * `list` must be null or a pointer returned by `ad_list_apps`. + */ +void ad_app_list_free(struct AdAppList *list); /** * Last-error lifetime — errno-style. @@ -425,33 +470,101 @@ AdResult ad_drag(const struct AdAdapter *adapter, const struct AdDragParams *par AdResult ad_mouse_event(const struct AdAdapter *adapter, const struct AdMouseEvent *event); /** + * Borrowed pointer to the image bytes; valid until the buffer is freed. + * Returns null if `buf` is null. + * * # Safety - * `adapter` and `target` must be valid. `out` must be writable. + * `buf` must be null or returned by `ad_screenshot`. + */ +const uint8_t *ad_image_buffer_data(const struct AdImageBuffer *buf); + +/** + * Byte length of the buffer returned by `ad_image_buffer_data`. + * Always consistent with the actual allocation (no C-mutable mismatch). + * + * # Safety + * `buf` must be null or returned by `ad_screenshot`. + */ +uint64_t ad_image_buffer_size(const struct AdImageBuffer *buf); + +/** + * Pixel width of the image. + * + * # Safety + * `buf` must be null or returned by `ad_screenshot`. + */ +uint32_t ad_image_buffer_width(const struct AdImageBuffer *buf); + +/** + * Pixel height of the image. + * + * # Safety + * `buf` must be null or returned by `ad_screenshot`. + */ +uint32_t ad_image_buffer_height(const struct AdImageBuffer *buf); + +/** + * Encoding format of the image bytes. Defaults to `PNG` on a null + * handle — callers must still null-check. + * + * # Safety + * `buf` must be null or returned by `ad_screenshot`. + */ +AdImageFormat ad_image_buffer_format(const struct AdImageBuffer *buf); + +/** + * Allocates and returns an opaque `AdImageBuffer`. The handle owns its + * byte buffer; inspect it through `ad_image_buffer_data` / + * `ad_image_buffer_size` / `ad_image_buffer_format` / `_width` / `_height` + * and free it with `ad_image_buffer_free`. + * + * # Safety + * `adapter` and `target` must be valid pointers. `out` must be a valid + * writable `*mut *mut AdImageBuffer`. On error `*out` is null and + * last-error is set. */ AdResult ad_screenshot(const struct AdAdapter *adapter, const struct AdScreenshotTarget *target, - struct AdImageBuffer *out); + struct AdImageBuffer **out); + +/** + * Frees the image buffer allocated by `ad_screenshot`. + * + * # Safety + * `buf` must be null or a pointer previously returned by `ad_screenshot`. + * Double-free is undefined behavior. + */ +void ad_image_buffer_free(struct AdImageBuffer *buf); /** * # Safety - * `img` must be null or point to an `AdImageBuffer` from `ad_screenshot`. + * `adapter` must be valid. `out` must be a valid writable + * `*mut *mut AdSurfaceList`. Success produces a list handle freed via + * `ad_surface_list_free`. */ -void ad_free_image(struct AdImageBuffer *img); +AdResult ad_list_surfaces(const struct AdAdapter *adapter, int32_t pid, struct AdSurfaceList **out); /** * # Safety - * `adapter` must be valid. `out` and `out_count` must be writable. + * `list` must be null or a pointer returned by `ad_list_surfaces`. */ -AdResult ad_list_surfaces(const struct AdAdapter *adapter, - int32_t pid, - struct AdSurfaceInfo **out, - uint32_t *out_count); +uint32_t ad_surface_list_count(const struct AdSurfaceList *list); /** + * Borrow a surface info entry. Null if `index` is out of range. + * * # Safety - * `surfaces` must be null or from `ad_list_surfaces`. + * `list` must be null or a pointer returned by `ad_list_surfaces`. */ -void ad_free_surfaces(struct AdSurfaceInfo *surfaces, uint32_t count); +const struct AdSurfaceInfo *ad_surface_list_get(const struct AdSurfaceList *list, uint32_t index); + +/** + * Frees the list and each entry's interior strings. + * + * # Safety + * `list` must be null or a pointer returned by `ad_list_surfaces`. + */ +void ad_surface_list_free(struct AdSurfaceList *list); /** * # Safety @@ -476,26 +589,55 @@ AdResult ad_get_tree(const struct AdAdapter *adapter, AdResult ad_focus_window(const struct AdAdapter *adapter, const struct AdWindowInfo *win); /** + * Releases the heap-allocated string fields (`id`, `title`, `app_name`) + * inside a single `AdWindowInfo` previously written by `ad_launch_app` + * or returned through a list accessor. Does not free the `AdWindowInfo` + * struct itself — that memory is owned by the caller's stack or by the + * enclosing list. + * + * Named `ad_release_window_fields` (not `ad_free_window`) to disambiguate + * from the now-removed list-free function and make the semantics clear + * in the header. + * * # Safety - * `win` must be null or point to a valid `AdWindowInfo`. + * `win` must be null or point to a valid `AdWindowInfo` whose string + * fields were allocated by this crate. Do not call on pointers inside + * an `AdWindowList` — free the list instead. */ -void ad_free_window(struct AdWindowInfo *win); +void ad_release_window_fields(struct AdWindowInfo *win); /** * # Safety - * `adapter` must be valid. `out` and `out_count` must be writable. + * `adapter` must be valid. `out` must be a valid writable + * `*mut *mut AdWindowList`. `app_filter` may be null or a C string. + * Success produces a list handle freed via `ad_window_list_free`. */ AdResult ad_list_windows(const struct AdAdapter *adapter, const char *app_filter, bool focused_only, - struct AdWindowInfo **out, - uint32_t *out_count); + struct AdWindowList **out); /** * # Safety - * `windows` must be null or from `ad_list_windows`. + * `list` must be null or a pointer returned by `ad_list_windows`. */ -void ad_free_windows(struct AdWindowInfo *windows, uint32_t count); +uint32_t ad_window_list_count(const struct AdWindowList *list); + +/** + * Borrow a window info entry. Null if `index` is out of range. + * + * # Safety + * `list` must be null or a pointer returned by `ad_list_windows`. + */ +const struct AdWindowInfo *ad_window_list_get(const struct AdWindowList *list, uint32_t index); + +/** + * Frees the list and each entry's interior strings. + * + * # Safety + * `list` must be null or a pointer returned by `ad_list_windows`. + */ +void ad_window_list_free(struct AdWindowList *list); /** * # Safety diff --git a/crates/ffi/src/apps/list.rs b/crates/ffi/src/apps/list.rs index 2ecc16c..bf95ff0 100644 --- a/crates/ffi/src/apps/list.rs +++ b/crates/ffi/src/apps/list.rs @@ -1,35 +1,30 @@ use crate::convert::app::{app_info_to_c, free_app_info_fields}; use crate::error::{set_last_error, AdResult}; use crate::ffi_try::{trap_panic, trap_panic_void}; -use crate::types::AdAppInfo; +use crate::types::{AdAppInfo, AdAppList}; use crate::AdAdapter; use std::ptr; /// # Safety /// `adapter` must be a valid pointer from `ad_adapter_create`. -/// `out` and `out_count` must be valid writable pointers. +/// `out` must be a valid writable `*mut *mut AdAppList`. +/// On success, `*out` is a newly-allocated opaque list freed with +/// `ad_app_list_free`. On error, `*out` is null and last-error is set. #[no_mangle] pub unsafe extern "C" fn ad_list_apps( adapter: *const AdAdapter, - out: *mut *mut AdAppInfo, - out_count: *mut u32, + out: *mut *mut AdAppList, ) -> AdResult { trap_panic(|| unsafe { *out = ptr::null_mut(); - *out_count = 0; - let adapter = &*adapter; match adapter.inner.list_apps() { Ok(apps) => { - let c_apps: Vec = apps.iter().map(app_info_to_c).collect(); - let count = c_apps.len() as u32; - if c_apps.is_empty() { - return AdResult::Ok; - } - let mut boxed = c_apps.into_boxed_slice(); - *out = boxed.as_mut_ptr(); - *out_count = count; - std::mem::forget(boxed); + let items: Vec = apps.iter().map(app_info_to_c).collect(); + let list = Box::new(AdAppList { + items: items.into_boxed_slice(), + }); + *out = Box::into_raw(list); AdResult::Ok } Err(e) => { @@ -41,20 +36,50 @@ pub unsafe extern "C" fn ad_list_apps( } /// # Safety -/// `apps` must be null or a pointer previously returned by `ad_list_apps`. +/// `list` must be null or a pointer returned by `ad_list_apps`. #[no_mangle] -pub unsafe extern "C" fn ad_free_apps(apps: *mut AdAppInfo, count: u32) { +pub unsafe extern "C" fn ad_app_list_count(list: *const AdAppList) -> u32 { + if list.is_null() { + return 0; + } + let list_ref: &AdAppList = unsafe { &*list }; + list_ref.items.len() as u32 +} + +/// Returns a borrowed pointer into the list; valid until the list is freed. +/// Out-of-range `index` returns null. +/// +/// # Safety +/// `list` must be null or a pointer returned by `ad_list_apps`. +#[no_mangle] +pub unsafe extern "C" fn ad_app_list_get( + list: *const AdAppList, + index: u32, +) -> *const AdAppInfo { + if list.is_null() { + return ptr::null(); + } + let list_ref: &AdAppList = unsafe { &*list }; + match list_ref.items.get(index as usize) { + Some(item) => item as *const AdAppInfo, + None => ptr::null(), + } +} + +/// Frees the list and every `AdAppInfo` it owns, including the interior +/// C-strings. +/// +/// # Safety +/// `list` must be null or a pointer returned by `ad_list_apps`. +#[no_mangle] +pub unsafe extern "C" fn ad_app_list_free(list: *mut AdAppList) { trap_panic_void(|| unsafe { - if apps.is_null() { + if list.is_null() { return; } - let slice = std::slice::from_raw_parts_mut(apps, count as usize); - for app in slice.iter_mut() { - free_app_info_fields(app); + let mut list = Box::from_raw(list); + for item in list.items.iter_mut() { + free_app_info_fields(item); } - drop(Box::from_raw(std::ptr::slice_from_raw_parts_mut( - apps, - count as usize, - ))); }) } diff --git a/crates/ffi/src/lib.rs b/crates/ffi/src/lib.rs index ec03bbf..b5698a6 100644 --- a/crates/ffi/src/lib.rs +++ b/crates/ffi/src/lib.rs @@ -53,9 +53,11 @@ pub use types::action::AdAction; pub use types::action_kind::AdActionKind; pub use types::action_result::AdActionResult; pub use types::app_info::AdAppInfo; +pub use types::app_list::AdAppList; pub use types::direction::AdDirection; pub use types::drag_params::AdDragParams; pub use types::element_state::AdElementState; +pub use types::find_query::AdFindQuery; pub use types::image_buffer::AdImageBuffer; pub use types::image_format::AdImageFormat; pub use types::key_combo::AdKeyCombo; @@ -66,6 +68,9 @@ pub use types::mouse_event_kind::AdMouseEventKind; pub use types::native_handle::AdNativeHandle; pub use types::node::AdNode; pub use types::node_tree::AdNodeTree; +pub use types::notification_filter::AdNotificationFilter; +pub use types::notification_info::AdNotificationInfo; +pub use types::notification_list::AdNotificationList; pub use types::point::AdPoint; pub use types::rect::AdRect; pub use types::ref_entry::AdRefEntry; @@ -74,7 +79,9 @@ pub use types::screenshot_target::AdScreenshotTarget; pub use types::scroll_params::AdScrollParams; pub use types::snapshot_surface::AdSnapshotSurface; pub use types::surface_info::AdSurfaceInfo; +pub use types::surface_list::AdSurfaceList; pub use types::tree_options::AdTreeOptions; pub use types::window_info::AdWindowInfo; +pub use types::window_list::AdWindowList; pub use types::window_op::AdWindowOp; pub use types::window_op_kind::AdWindowOpKind; diff --git a/crates/ffi/src/screenshot/accessors.rs b/crates/ffi/src/screenshot/accessors.rs new file mode 100644 index 0000000..2aac1b0 --- /dev/null +++ b/crates/ffi/src/screenshot/accessors.rs @@ -0,0 +1,70 @@ +use crate::types::{AdImageBuffer, AdImageFormat}; +use std::ptr; + +/// Borrowed pointer to the image bytes; valid until the buffer is freed. +/// Returns null if `buf` is null. +/// +/// # Safety +/// `buf` must be null or returned by `ad_screenshot`. +#[no_mangle] +pub unsafe extern "C" fn ad_image_buffer_data(buf: *const AdImageBuffer) -> *const u8 { + if buf.is_null() { + return ptr::null(); + } + let buf_ref: &AdImageBuffer = unsafe { &*buf }; + buf_ref.data.as_ptr() +} + +/// Byte length of the buffer returned by `ad_image_buffer_data`. +/// Always consistent with the actual allocation (no C-mutable mismatch). +/// +/// # Safety +/// `buf` must be null or returned by `ad_screenshot`. +#[no_mangle] +pub unsafe extern "C" fn ad_image_buffer_size(buf: *const AdImageBuffer) -> u64 { + if buf.is_null() { + return 0; + } + let buf_ref: &AdImageBuffer = unsafe { &*buf }; + buf_ref.data.len() as u64 +} + +/// Pixel width of the image. +/// +/// # Safety +/// `buf` must be null or returned by `ad_screenshot`. +#[no_mangle] +pub unsafe extern "C" fn ad_image_buffer_width(buf: *const AdImageBuffer) -> u32 { + if buf.is_null() { + return 0; + } + let buf_ref: &AdImageBuffer = unsafe { &*buf }; + buf_ref.width +} + +/// Pixel height of the image. +/// +/// # Safety +/// `buf` must be null or returned by `ad_screenshot`. +#[no_mangle] +pub unsafe extern "C" fn ad_image_buffer_height(buf: *const AdImageBuffer) -> u32 { + if buf.is_null() { + return 0; + } + let buf_ref: &AdImageBuffer = unsafe { &*buf }; + buf_ref.height +} + +/// Encoding format of the image bytes. Defaults to `PNG` on a null +/// handle — callers must still null-check. +/// +/// # Safety +/// `buf` must be null or returned by `ad_screenshot`. +#[no_mangle] +pub unsafe extern "C" fn ad_image_buffer_format(buf: *const AdImageBuffer) -> AdImageFormat { + if buf.is_null() { + return AdImageFormat::Png; + } + let buf_ref: &AdImageBuffer = unsafe { &*buf }; + buf_ref.format +} diff --git a/crates/ffi/src/screenshot/capture.rs b/crates/ffi/src/screenshot/capture.rs index 94350d9..4d77c20 100644 --- a/crates/ffi/src/screenshot/capture.rs +++ b/crates/ffi/src/screenshot/capture.rs @@ -4,18 +4,26 @@ use crate::ffi_try::trap_panic; use crate::types::{AdImageBuffer, AdImageFormat, AdScreenshotKind, AdScreenshotTarget}; use crate::AdAdapter; use agent_desktop_core::adapter::{ImageFormat, ScreenshotTarget as CoreScreenshotTarget}; +use std::ptr; +/// Allocates and returns an opaque `AdImageBuffer`. The handle owns its +/// byte buffer; inspect it through `ad_image_buffer_data` / +/// `ad_image_buffer_size` / `ad_image_buffer_format` / `_width` / `_height` +/// and free it with `ad_image_buffer_free`. +/// /// # Safety -/// `adapter` and `target` must be valid. `out` must be writable. +/// `adapter` and `target` must be valid pointers. `out` must be a valid +/// writable `*mut *mut AdImageBuffer`. On error `*out` is null and +/// last-error is set. #[no_mangle] pub unsafe extern "C" fn ad_screenshot( adapter: *const AdAdapter, target: *const AdScreenshotTarget, - out: *mut AdImageBuffer, + out: *mut *mut AdImageBuffer, ) -> AdResult { trap_panic(|| unsafe { crate::main_thread::debug_assert_main_thread(); - *out = std::mem::zeroed(); + *out = ptr::null_mut(); let adapter = &*adapter; let t = &*target; let kind = match AdScreenshotKind::from_c(enum_raw_i32(&t.kind)) { @@ -36,21 +44,16 @@ pub unsafe extern "C" fn ad_screenshot( match adapter.inner.screenshot(core_target) { Ok(img) => { - let data_len = img.data.len() as u64; - let mut boxed = img.data.into_boxed_slice(); - let data_ptr = boxed.as_mut_ptr(); - std::mem::forget(boxed); - - *out = AdImageBuffer { - data: data_ptr, - data_len, + let buffer = Box::new(AdImageBuffer { + data: img.data.into_boxed_slice(), + width: img.width, + height: img.height, format: match img.format { ImageFormat::Png => AdImageFormat::Png, ImageFormat::Jpg => AdImageFormat::Jpg, }, - width: img.width, - height: img.height, - }; + }); + *out = Box::into_raw(buffer); AdResult::Ok } Err(e) => { diff --git a/crates/ffi/src/screenshot/free.rs b/crates/ffi/src/screenshot/free.rs index 95cb6c1..4e24dcb 100644 --- a/crates/ffi/src/screenshot/free.rs +++ b/crates/ffi/src/screenshot/free.rs @@ -1,22 +1,17 @@ use crate::ffi_try::trap_panic_void; use crate::types::AdImageBuffer; +/// Frees the image buffer allocated by `ad_screenshot`. +/// /// # Safety -/// `img` must be null or point to an `AdImageBuffer` from `ad_screenshot`. +/// `buf` must be null or a pointer previously returned by `ad_screenshot`. +/// Double-free is undefined behavior. #[no_mangle] -pub unsafe extern "C" fn ad_free_image(img: *mut AdImageBuffer) { +pub unsafe extern "C" fn ad_image_buffer_free(buf: *mut AdImageBuffer) { trap_panic_void(|| unsafe { - if img.is_null() { + if buf.is_null() { return; } - let i = &mut *img; - if !i.data.is_null() { - drop(Box::from_raw(std::ptr::slice_from_raw_parts_mut( - i.data as *mut u8, - i.data_len as usize, - ))); - i.data = std::ptr::null(); - i.data_len = 0; - } + drop(Box::from_raw(buf)); }) } diff --git a/crates/ffi/src/screenshot/mod.rs b/crates/ffi/src/screenshot/mod.rs index db5e0c5..b307f2d 100644 --- a/crates/ffi/src/screenshot/mod.rs +++ b/crates/ffi/src/screenshot/mod.rs @@ -1,2 +1,3 @@ +pub(crate) mod accessors; pub(crate) mod capture; pub(crate) mod free; diff --git a/crates/ffi/src/surfaces/list.rs b/crates/ffi/src/surfaces/list.rs index caaa9b5..5de3f4e 100644 --- a/crates/ffi/src/surfaces/list.rs +++ b/crates/ffi/src/surfaces/list.rs @@ -1,36 +1,30 @@ use crate::convert::surface::{free_surface_info_fields, surface_info_to_c}; use crate::error::{set_last_error, AdResult}; use crate::ffi_try::{trap_panic, trap_panic_void}; -use crate::types::AdSurfaceInfo; +use crate::types::{AdSurfaceInfo, AdSurfaceList}; use crate::AdAdapter; use std::ptr; /// # Safety -/// `adapter` must be valid. `out` and `out_count` must be writable. +/// `adapter` must be valid. `out` must be a valid writable +/// `*mut *mut AdSurfaceList`. Success produces a list handle freed via +/// `ad_surface_list_free`. #[no_mangle] pub unsafe extern "C" fn ad_list_surfaces( adapter: *const AdAdapter, pid: i32, - out: *mut *mut AdSurfaceInfo, - out_count: *mut u32, + out: *mut *mut AdSurfaceList, ) -> AdResult { trap_panic(|| unsafe { *out = ptr::null_mut(); - *out_count = 0; - let adapter = &*adapter; match adapter.inner.list_surfaces(pid) { Ok(surfaces) => { - let c_surfaces: Vec = - surfaces.iter().map(surface_info_to_c).collect(); - let count = c_surfaces.len() as u32; - if c_surfaces.is_empty() { - return AdResult::Ok; - } - let mut boxed = c_surfaces.into_boxed_slice(); - *out = boxed.as_mut_ptr(); - *out_count = count; - std::mem::forget(boxed); + let items: Vec = surfaces.iter().map(surface_info_to_c).collect(); + let list = Box::new(AdSurfaceList { + items: items.into_boxed_slice(), + }); + *out = Box::into_raw(list); AdResult::Ok } Err(e) => { @@ -42,20 +36,48 @@ pub unsafe extern "C" fn ad_list_surfaces( } /// # Safety -/// `surfaces` must be null or from `ad_list_surfaces`. +/// `list` must be null or a pointer returned by `ad_list_surfaces`. #[no_mangle] -pub unsafe extern "C" fn ad_free_surfaces(surfaces: *mut AdSurfaceInfo, count: u32) { +pub unsafe extern "C" fn ad_surface_list_count(list: *const AdSurfaceList) -> u32 { + if list.is_null() { + return 0; + } + let list_ref: &AdSurfaceList = unsafe { &*list }; + list_ref.items.len() as u32 +} + +/// Borrow a surface info entry. Null if `index` is out of range. +/// +/// # Safety +/// `list` must be null or a pointer returned by `ad_list_surfaces`. +#[no_mangle] +pub unsafe extern "C" fn ad_surface_list_get( + list: *const AdSurfaceList, + index: u32, +) -> *const AdSurfaceInfo { + if list.is_null() { + return ptr::null(); + } + let list_ref: &AdSurfaceList = unsafe { &*list }; + match list_ref.items.get(index as usize) { + Some(item) => item as *const AdSurfaceInfo, + None => ptr::null(), + } +} + +/// Frees the list and each entry's interior strings. +/// +/// # Safety +/// `list` must be null or a pointer returned by `ad_list_surfaces`. +#[no_mangle] +pub unsafe extern "C" fn ad_surface_list_free(list: *mut AdSurfaceList) { trap_panic_void(|| unsafe { - if surfaces.is_null() { + if list.is_null() { return; } - let slice = std::slice::from_raw_parts_mut(surfaces, count as usize); - for s in slice.iter_mut() { - free_surface_info_fields(s); + let mut list = Box::from_raw(list); + for item in list.items.iter_mut() { + free_surface_info_fields(item); } - drop(Box::from_raw(std::ptr::slice_from_raw_parts_mut( - surfaces, - count as usize, - ))); }) } diff --git a/crates/ffi/src/types/app_list.rs b/crates/ffi/src/types/app_list.rs new file mode 100644 index 0000000..5118ade --- /dev/null +++ b/crates/ffi/src/types/app_list.rs @@ -0,0 +1,7 @@ +use crate::types::app_info::AdAppInfo; + +/// Opaque list handle emitted by `ad_list_apps`. See +/// [`crate::types::window_list::AdWindowList`] for the pattern. +pub struct AdAppList { + pub(crate) items: Box<[AdAppInfo]>, +} diff --git a/crates/ffi/src/types/find_query.rs b/crates/ffi/src/types/find_query.rs new file mode 100644 index 0000000..6f80efb --- /dev/null +++ b/crates/ffi/src/types/find_query.rs @@ -0,0 +1,8 @@ +use std::os::raw::c_char; + +#[repr(C)] +pub struct AdFindQuery { + pub role: *const c_char, + pub name_substring: *const c_char, + pub value_substring: *const c_char, +} diff --git a/crates/ffi/src/types/image_buffer.rs b/crates/ffi/src/types/image_buffer.rs index 3896d86..f225a89 100644 --- a/crates/ffi/src/types/image_buffer.rs +++ b/crates/ffi/src/types/image_buffer.rs @@ -1,10 +1,13 @@ use crate::types::image_format::AdImageFormat; -#[repr(C)] +/// Opaque image-buffer handle returned by `ad_screenshot`. The backing +/// byte buffer and its length live inside the Rust-owned struct — a +/// consumer cannot accidentally desynchronize the pair and trigger a +/// heap-corruption double-free. Walk it through `ad_image_buffer_*` +/// accessors and free it with `ad_image_buffer_free`. pub struct AdImageBuffer { - pub data: *const u8, - pub data_len: u64, - pub format: AdImageFormat, - pub width: u32, - pub height: u32, + pub(crate) data: Box<[u8]>, + pub(crate) width: u32, + pub(crate) height: u32, + pub(crate) format: AdImageFormat, } diff --git a/crates/ffi/src/types/mod.rs b/crates/ffi/src/types/mod.rs index 93fb334..a1ffd95 100644 --- a/crates/ffi/src/types/mod.rs +++ b/crates/ffi/src/types/mod.rs @@ -2,9 +2,11 @@ pub mod action; pub mod action_kind; pub mod action_result; pub mod app_info; +pub mod app_list; pub mod direction; pub mod drag_params; pub mod element_state; +pub mod find_query; pub mod image_buffer; pub mod image_format; pub mod key_combo; @@ -15,6 +17,9 @@ pub mod mouse_event_kind; pub mod native_handle; pub mod node; pub mod node_tree; +pub mod notification_filter; +pub mod notification_info; +pub mod notification_list; pub mod point; pub mod rect; pub mod ref_entry; @@ -23,8 +28,10 @@ pub mod screenshot_target; pub mod scroll_params; pub mod snapshot_surface; pub mod surface_info; +pub mod surface_list; pub mod tree_options; pub mod window_info; +pub mod window_list; pub mod window_op; pub mod window_op_kind; @@ -32,9 +39,11 @@ pub use action::AdAction; pub use action_kind::AdActionKind; pub use action_result::AdActionResult; pub use app_info::AdAppInfo; +pub use app_list::AdAppList; pub use direction::AdDirection; pub use drag_params::AdDragParams; pub use element_state::AdElementState; +pub use find_query::AdFindQuery; pub use image_buffer::AdImageBuffer; pub use image_format::AdImageFormat; pub use key_combo::AdKeyCombo; @@ -45,6 +54,9 @@ pub use mouse_event_kind::AdMouseEventKind; pub use native_handle::AdNativeHandle; pub use node::AdNode; pub use node_tree::AdNodeTree; +pub use notification_filter::AdNotificationFilter; +pub use notification_info::AdNotificationInfo; +pub use notification_list::AdNotificationList; pub use point::AdPoint; pub use rect::AdRect; pub use ref_entry::AdRefEntry; @@ -53,7 +65,9 @@ pub use screenshot_target::AdScreenshotTarget; pub use scroll_params::AdScrollParams; pub use snapshot_surface::AdSnapshotSurface; pub use surface_info::AdSurfaceInfo; +pub use surface_list::AdSurfaceList; pub use tree_options::AdTreeOptions; pub use window_info::AdWindowInfo; +pub use window_list::AdWindowList; pub use window_op::AdWindowOp; pub use window_op_kind::AdWindowOpKind; diff --git a/crates/ffi/src/types/notification_filter.rs b/crates/ffi/src/types/notification_filter.rs new file mode 100644 index 0000000..3a296ee --- /dev/null +++ b/crates/ffi/src/types/notification_filter.rs @@ -0,0 +1,9 @@ +use std::os::raw::c_char; + +#[repr(C)] +pub struct AdNotificationFilter { + pub app: *const c_char, + pub text: *const c_char, + pub limit: u32, + pub has_limit: bool, +} diff --git a/crates/ffi/src/types/notification_info.rs b/crates/ffi/src/types/notification_info.rs new file mode 100644 index 0000000..2965036 --- /dev/null +++ b/crates/ffi/src/types/notification_info.rs @@ -0,0 +1,11 @@ +use std::os::raw::c_char; + +#[repr(C)] +pub struct AdNotificationInfo { + pub index: u32, + pub app_name: *const c_char, + pub title: *const c_char, + pub body: *const c_char, + pub actions: *mut *mut c_char, + pub action_count: u32, +} diff --git a/crates/ffi/src/types/notification_list.rs b/crates/ffi/src/types/notification_list.rs new file mode 100644 index 0000000..14ba200 --- /dev/null +++ b/crates/ffi/src/types/notification_list.rs @@ -0,0 +1,6 @@ +use crate::types::notification_info::AdNotificationInfo; + +pub struct AdNotificationList { + #[allow(dead_code)] // populated + read by the Unit 8 notifications module + pub(crate) items: Box<[AdNotificationInfo]>, +} diff --git a/crates/ffi/src/types/surface_list.rs b/crates/ffi/src/types/surface_list.rs new file mode 100644 index 0000000..60f765b --- /dev/null +++ b/crates/ffi/src/types/surface_list.rs @@ -0,0 +1,7 @@ +use crate::types::surface_info::AdSurfaceInfo; + +/// Opaque list handle emitted by `ad_list_surfaces`. See +/// [`crate::types::window_list::AdWindowList`] for the pattern. +pub struct AdSurfaceList { + pub(crate) items: Box<[AdSurfaceInfo]>, +} diff --git a/crates/ffi/src/types/window_list.rs b/crates/ffi/src/types/window_list.rs new file mode 100644 index 0000000..0e40fed --- /dev/null +++ b/crates/ffi/src/types/window_list.rs @@ -0,0 +1,13 @@ +use crate::types::window_info::AdWindowInfo; + +/// Opaque list handle emitted by `ad_list_windows`. +/// +/// The struct intentionally has no `#[repr(C)]` so cbindgen emits a +/// forward declaration only (`typedef struct AdWindowList AdWindowList;`). +/// Consumers cannot read the backing pointer or length and cannot +/// construct a count mismatch — they walk the list through +/// `ad_window_list_count`, `ad_window_list_get`, and free it with +/// `ad_window_list_free`. +pub struct AdWindowList { + pub(crate) items: Box<[AdWindowInfo]>, +} diff --git a/crates/ffi/src/windows/free_one.rs b/crates/ffi/src/windows/free_one.rs index 69fa233..7683d86 100644 --- a/crates/ffi/src/windows/free_one.rs +++ b/crates/ffi/src/windows/free_one.rs @@ -2,10 +2,22 @@ use crate::convert::window::free_window_info_fields; use crate::ffi_try::trap_panic_void; use crate::types::AdWindowInfo; +/// Releases the heap-allocated string fields (`id`, `title`, `app_name`) +/// inside a single `AdWindowInfo` previously written by `ad_launch_app` +/// or returned through a list accessor. Does not free the `AdWindowInfo` +/// struct itself — that memory is owned by the caller's stack or by the +/// enclosing list. +/// +/// Named `ad_release_window_fields` (not `ad_free_window`) to disambiguate +/// from the now-removed list-free function and make the semantics clear +/// in the header. +/// /// # Safety -/// `win` must be null or point to a valid `AdWindowInfo`. +/// `win` must be null or point to a valid `AdWindowInfo` whose string +/// fields were allocated by this crate. Do not call on pointers inside +/// an `AdWindowList` — free the list instead. #[no_mangle] -pub unsafe extern "C" fn ad_free_window(win: *mut AdWindowInfo) { +pub unsafe extern "C" fn ad_release_window_fields(win: *mut AdWindowInfo) { trap_panic_void(|| unsafe { if win.is_null() { return; diff --git a/crates/ffi/src/windows/list.rs b/crates/ffi/src/windows/list.rs index f62eb08..5bfd570 100644 --- a/crates/ffi/src/windows/list.rs +++ b/crates/ffi/src/windows/list.rs @@ -2,25 +2,25 @@ use crate::convert::string::c_to_string; use crate::convert::window::{free_window_info_fields, window_info_to_c}; use crate::error::{set_last_error, AdResult}; use crate::ffi_try::{trap_panic, trap_panic_void}; -use crate::types::AdWindowInfo; +use crate::types::{AdWindowInfo, AdWindowList}; use crate::AdAdapter; use agent_desktop_core::adapter::WindowFilter; use std::os::raw::c_char; use std::ptr; /// # Safety -/// `adapter` must be valid. `out` and `out_count` must be writable. +/// `adapter` must be valid. `out` must be a valid writable +/// `*mut *mut AdWindowList`. `app_filter` may be null or a C string. +/// Success produces a list handle freed via `ad_window_list_free`. #[no_mangle] pub unsafe extern "C" fn ad_list_windows( adapter: *const AdAdapter, app_filter: *const c_char, focused_only: bool, - out: *mut *mut AdWindowInfo, - out_count: *mut u32, + out: *mut *mut AdWindowList, ) -> AdResult { trap_panic(|| unsafe { *out = ptr::null_mut(); - *out_count = 0; let adapter = &*adapter; let filter = WindowFilter { focused_only, @@ -28,15 +28,11 @@ pub unsafe extern "C" fn ad_list_windows( }; match adapter.inner.list_windows(&filter) { Ok(windows) => { - let c_wins: Vec = windows.iter().map(window_info_to_c).collect(); - let count = c_wins.len() as u32; - if c_wins.is_empty() { - return AdResult::Ok; - } - let mut boxed = c_wins.into_boxed_slice(); - *out = boxed.as_mut_ptr(); - *out_count = count; - std::mem::forget(boxed); + let items: Vec = windows.iter().map(window_info_to_c).collect(); + let list = Box::new(AdWindowList { + items: items.into_boxed_slice(), + }); + *out = Box::into_raw(list); AdResult::Ok } Err(e) => { @@ -48,20 +44,48 @@ pub unsafe extern "C" fn ad_list_windows( } /// # Safety -/// `windows` must be null or from `ad_list_windows`. +/// `list` must be null or a pointer returned by `ad_list_windows`. #[no_mangle] -pub unsafe extern "C" fn ad_free_windows(windows: *mut AdWindowInfo, count: u32) { +pub unsafe extern "C" fn ad_window_list_count(list: *const AdWindowList) -> u32 { + if list.is_null() { + return 0; + } + let list_ref: &AdWindowList = unsafe { &*list }; + list_ref.items.len() as u32 +} + +/// Borrow a window info entry. Null if `index` is out of range. +/// +/// # Safety +/// `list` must be null or a pointer returned by `ad_list_windows`. +#[no_mangle] +pub unsafe extern "C" fn ad_window_list_get( + list: *const AdWindowList, + index: u32, +) -> *const AdWindowInfo { + if list.is_null() { + return ptr::null(); + } + let list_ref: &AdWindowList = unsafe { &*list }; + match list_ref.items.get(index as usize) { + Some(item) => item as *const AdWindowInfo, + None => ptr::null(), + } +} + +/// Frees the list and each entry's interior strings. +/// +/// # Safety +/// `list` must be null or a pointer returned by `ad_list_windows`. +#[no_mangle] +pub unsafe extern "C" fn ad_window_list_free(list: *mut AdWindowList) { trap_panic_void(|| unsafe { - if windows.is_null() { + if list.is_null() { return; } - let slice = std::slice::from_raw_parts_mut(windows, count as usize); - for w in slice.iter_mut() { - free_window_info_fields(w); + let mut list = Box::from_raw(list); + for item in list.items.iter_mut() { + free_window_info_fields(item); } - drop(Box::from_raw(std::ptr::slice_from_raw_parts_mut( - windows, - count as usize, - ))); }) }