diff --git a/crates/ffi/Cargo.toml b/crates/ffi/Cargo.toml index 1ec3eb5..c6af817 100644 --- a/crates/ffi/Cargo.toml +++ b/crates/ffi/Cargo.toml @@ -6,7 +6,7 @@ license.workspace = true publish = false [lib] -crate-type = ["cdylib"] +crate-type = ["cdylib", "rlib"] [dependencies] agent-desktop-core.workspace = true diff --git a/crates/ffi/include/agent_desktop.h b/crates/ffi/include/agent_desktop.h index 8ec200b..7f7ef05 100644 --- a/crates/ffi/include/agent_desktop.h +++ b/crates/ffi/include/agent_desktop.h @@ -321,6 +321,23 @@ AdResult ad_list_apps(const struct AdAdapter *adapter, struct AdAppInfo **out, u */ void ad_free_apps(struct AdAppInfo *apps, uint32_t count); +/** + * Last-error lifetime — errno-style. + * + * The pointer returned by `ad_last_error_message`, + * `ad_last_error_suggestion`, and `ad_last_error_platform_detail` + * remains valid across any number of subsequent **successful** FFI + * calls on the same thread. Only the next FFI call that itself **fails** + * (returns a non-`AD_RESULT_OK` code) invalidates the previous pointers. + * + * Consumers can therefore read an error once, cache the pointer, and + * keep reading it back across follow-up work that clears or re-fetches + * state before handing control to the user. + * + * This matches the POSIX `errno` / `strerror` contract and is scoped + * per-thread via thread-local storage — Thread A's last-error never + * leaks to Thread B. + */ AdResult ad_last_error_code(void); const char *ad_last_error_message(void); diff --git a/crates/ffi/src/actions/execute.rs b/crates/ffi/src/actions/execute.rs index 692cd3d..e49cf01 100644 --- a/crates/ffi/src/actions/execute.rs +++ b/crates/ffi/src/actions/execute.rs @@ -37,7 +37,6 @@ pub unsafe extern "C" fn ad_execute_action( match adapter.inner.execute_action(&native_handle, core_action) { Ok(result) => { *out = action_result_to_c(&result); - error::clear_last_error(); AdResult::Ok } Err(e) => { diff --git a/crates/ffi/src/actions/resolve.rs b/crates/ffi/src/actions/resolve.rs index 260f852..4605f76 100644 --- a/crates/ffi/src/actions/resolve.rs +++ b/crates/ffi/src/actions/resolve.rs @@ -49,7 +49,6 @@ pub unsafe extern "C" fn ad_resolve_element( match adapter.inner.resolve_element(&core_entry) { Ok(handle) => { (*out).ptr = handle.as_raw(); - error::clear_last_error(); AdResult::Ok } Err(e) => { diff --git a/crates/ffi/src/adapter.rs b/crates/ffi/src/adapter.rs index f953209..703d500 100644 --- a/crates/ffi/src/adapter.rs +++ b/crates/ffi/src/adapter.rs @@ -58,10 +58,7 @@ pub unsafe extern "C" fn ad_check_permissions(adapter: *const AdAdapter) -> AdRe trap_panic(|| { let adapter = unsafe { &*adapter }; match adapter.inner.check_permissions() { - agent_desktop_core::adapter::PermissionStatus::Granted => { - error::clear_last_error(); - AdResult::Ok - } + agent_desktop_core::adapter::PermissionStatus::Granted => AdResult::Ok, agent_desktop_core::adapter::PermissionStatus::Denied { suggestion } => { error::set_last_error( &agent_desktop_core::error::AdapterError::new( diff --git a/crates/ffi/src/apps/close.rs b/crates/ffi/src/apps/close.rs index 725f8b1..f63b10f 100644 --- a/crates/ffi/src/apps/close.rs +++ b/crates/ffi/src/apps/close.rs @@ -1,5 +1,5 @@ use crate::convert::string::c_to_string; -use crate::error::{clear_last_error, set_last_error, AdResult}; +use crate::error::{set_last_error, AdResult}; use crate::ffi_try::trap_panic; use crate::AdAdapter; use std::os::raw::c_char; @@ -26,10 +26,7 @@ pub unsafe extern "C" fn ad_close_app( }; match adapter.inner.close_app(&id_str, force) { - Ok(()) => { - clear_last_error(); - AdResult::Ok - } + Ok(()) => AdResult::Ok, Err(e) => { set_last_error(&e); crate::error::last_error_code() diff --git a/crates/ffi/src/apps/launch.rs b/crates/ffi/src/apps/launch.rs index 386911b..5795f92 100644 --- a/crates/ffi/src/apps/launch.rs +++ b/crates/ffi/src/apps/launch.rs @@ -1,6 +1,6 @@ use crate::convert::string::c_to_string; use crate::convert::window::window_info_to_c; -use crate::error::{clear_last_error, set_last_error, AdResult}; +use crate::error::{set_last_error, AdResult}; use crate::ffi_try::trap_panic; use crate::types::AdWindowInfo; use crate::AdAdapter; @@ -30,7 +30,6 @@ pub unsafe extern "C" fn ad_launch_app( match adapter.inner.launch_app(&id_str, timeout_ms) { Ok(win) => { - clear_last_error(); *out = window_info_to_c(&win); AdResult::Ok } diff --git a/crates/ffi/src/apps/list.rs b/crates/ffi/src/apps/list.rs index 42be0ae..2ecc16c 100644 --- a/crates/ffi/src/apps/list.rs +++ b/crates/ffi/src/apps/list.rs @@ -1,5 +1,5 @@ use crate::convert::app::{app_info_to_c, free_app_info_fields}; -use crate::error::{clear_last_error, set_last_error, AdResult}; +use crate::error::{set_last_error, AdResult}; use crate::ffi_try::{trap_panic, trap_panic_void}; use crate::types::AdAppInfo; use crate::AdAdapter; @@ -21,7 +21,6 @@ pub unsafe extern "C" fn ad_list_apps( let adapter = &*adapter; match adapter.inner.list_apps() { Ok(apps) => { - clear_last_error(); let c_apps: Vec = apps.iter().map(app_info_to_c).collect(); let count = c_apps.len() as u32; if c_apps.is_empty() { diff --git a/crates/ffi/src/error.rs b/crates/ffi/src/error.rs index 6f65e07..ba6a4af 100644 --- a/crates/ffi/src/error.rs +++ b/crates/ffi/src/error.rs @@ -93,6 +93,7 @@ pub(crate) fn set_last_error(err: &AdapterError) { }); } +#[cfg(test)] pub(crate) fn clear_last_error() { LAST_ERROR.with(|cell| { *cell.borrow_mut() = None; @@ -121,6 +122,21 @@ pub(crate) fn last_error_code() -> AdResult { }) } +/// Last-error lifetime — errno-style. +/// +/// The pointer returned by `ad_last_error_message`, +/// `ad_last_error_suggestion`, and `ad_last_error_platform_detail` +/// remains valid across any number of subsequent **successful** FFI +/// calls on the same thread. Only the next FFI call that itself **fails** +/// (returns a non-`AD_RESULT_OK` code) invalidates the previous pointers. +/// +/// Consumers can therefore read an error once, cache the pointer, and +/// keep reading it back across follow-up work that clears or re-fetches +/// state before handing control to the user. +/// +/// This matches the POSIX `errno` / `strerror` contract and is scoped +/// per-thread via thread-local storage — Thread A's last-error never +/// leaks to Thread B. #[no_mangle] pub extern "C" fn ad_last_error_code() -> AdResult { crate::ffi_try::trap_panic(last_error_code) diff --git a/crates/ffi/src/input/clipboard.rs b/crates/ffi/src/input/clipboard.rs index e546b2f..67cec04 100644 --- a/crates/ffi/src/input/clipboard.rs +++ b/crates/ffi/src/input/clipboard.rs @@ -19,7 +19,6 @@ pub unsafe extern "C" fn ad_get_clipboard( match adapter.inner.get_clipboard() { Ok(text) => { *out = string_to_c(&text); - error::clear_last_error(); AdResult::Ok } Err(e) => { @@ -52,10 +51,7 @@ pub unsafe extern "C" fn ad_set_clipboard( } }; match adapter.inner.set_clipboard(&text) { - Ok(()) => { - error::clear_last_error(); - AdResult::Ok - } + Ok(()) => AdResult::Ok, Err(e) => { error::set_last_error(&e); error::last_error_code() @@ -72,10 +68,7 @@ pub unsafe extern "C" fn ad_clear_clipboard(adapter: *const AdAdapter) -> AdResu trap_panic(|| unsafe { let adapter = &*adapter; match adapter.inner.clear_clipboard() { - Ok(()) => { - error::clear_last_error(); - AdResult::Ok - } + Ok(()) => AdResult::Ok, Err(e) => { error::set_last_error(&e); error::last_error_code() diff --git a/crates/ffi/src/input/drag.rs b/crates/ffi/src/input/drag.rs index 428fc87..e8fa3d5 100644 --- a/crates/ffi/src/input/drag.rs +++ b/crates/ffi/src/input/drag.rs @@ -32,10 +32,7 @@ pub unsafe extern "C" fn ad_drag( }, }; match adapter.inner.drag(core_params) { - Ok(()) => { - error::clear_last_error(); - AdResult::Ok - } + Ok(()) => AdResult::Ok, Err(e) => { error::set_last_error(&e); error::last_error_code() diff --git a/crates/ffi/src/input/mouse.rs b/crates/ffi/src/input/mouse.rs index b3bb0a0..572a9e8 100644 --- a/crates/ffi/src/input/mouse.rs +++ b/crates/ffi/src/input/mouse.rs @@ -46,10 +46,7 @@ pub unsafe extern "C" fn ad_mouse_event( button, }; match adapter.inner.mouse_event(core_event) { - Ok(()) => { - error::clear_last_error(); - AdResult::Ok - } + Ok(()) => AdResult::Ok, Err(e) => { error::set_last_error(&e); error::last_error_code() diff --git a/crates/ffi/src/screenshot/capture.rs b/crates/ffi/src/screenshot/capture.rs index 5565332..cc45938 100644 --- a/crates/ffi/src/screenshot/capture.rs +++ b/crates/ffi/src/screenshot/capture.rs @@ -1,4 +1,4 @@ -use crate::error::{clear_last_error, set_last_error, AdResult}; +use crate::error::{set_last_error, AdResult}; use crate::ffi_try::trap_panic; use crate::types::{AdImageBuffer, AdImageFormat, AdScreenshotKind, AdScreenshotTarget}; use crate::AdAdapter; @@ -23,7 +23,6 @@ pub unsafe extern "C" fn ad_screenshot( match adapter.inner.screenshot(core_target) { Ok(img) => { - clear_last_error(); let data_len = img.data.len() as u64; let mut boxed = img.data.into_boxed_slice(); let data_ptr = boxed.as_mut_ptr(); diff --git a/crates/ffi/src/surfaces/list.rs b/crates/ffi/src/surfaces/list.rs index 594572a..caaa9b5 100644 --- a/crates/ffi/src/surfaces/list.rs +++ b/crates/ffi/src/surfaces/list.rs @@ -1,5 +1,5 @@ use crate::convert::surface::{free_surface_info_fields, surface_info_to_c}; -use crate::error::{clear_last_error, set_last_error, AdResult}; +use crate::error::{set_last_error, AdResult}; use crate::ffi_try::{trap_panic, trap_panic_void}; use crate::types::AdSurfaceInfo; use crate::AdAdapter; @@ -21,7 +21,6 @@ pub unsafe extern "C" fn ad_list_surfaces( let adapter = &*adapter; match adapter.inner.list_surfaces(pid) { Ok(surfaces) => { - clear_last_error(); let c_surfaces: Vec = surfaces.iter().map(surface_info_to_c).collect(); let count = c_surfaces.len() as u32; diff --git a/crates/ffi/src/tree/get.rs b/crates/ffi/src/tree/get.rs index c1d4516..bbb5cf1 100644 --- a/crates/ffi/src/tree/get.rs +++ b/crates/ffi/src/tree/get.rs @@ -1,4 +1,4 @@ -use crate::error::{clear_last_error, set_last_error, AdResult}; +use crate::error::{set_last_error, AdResult}; use crate::ffi_try::trap_panic; use crate::tree::flatten::flatten_tree; use crate::types::{AdNodeTree, AdTreeOptions, AdWindowInfo}; @@ -33,7 +33,6 @@ pub unsafe extern "C" fn ad_get_tree( match adapter.inner.get_tree(&core_win, &core_opts) { Ok(tree) => { - clear_last_error(); unsafe { *out = flatten_tree(&tree) }; AdResult::Ok } diff --git a/crates/ffi/src/windows/focus.rs b/crates/ffi/src/windows/focus.rs index 1fc9087..6fcf682 100644 --- a/crates/ffi/src/windows/focus.rs +++ b/crates/ffi/src/windows/focus.rs @@ -1,4 +1,4 @@ -use crate::error::{clear_last_error, set_last_error, AdResult}; +use crate::error::{set_last_error, AdResult}; use crate::ffi_try::trap_panic; use crate::types::AdWindowInfo; use crate::windows::to_core::ad_window_to_core; @@ -15,10 +15,7 @@ pub unsafe extern "C" fn ad_focus_window( let adapter = &*adapter; let core_win = ad_window_to_core(&*win); match adapter.inner.focus_window(&core_win) { - Ok(()) => { - clear_last_error(); - AdResult::Ok - } + Ok(()) => AdResult::Ok, Err(e) => { set_last_error(&e); crate::error::last_error_code() diff --git a/crates/ffi/src/windows/list.rs b/crates/ffi/src/windows/list.rs index 1228d0a..056af43 100644 --- a/crates/ffi/src/windows/list.rs +++ b/crates/ffi/src/windows/list.rs @@ -1,6 +1,6 @@ use crate::convert::string::c_to_string; use crate::convert::window::{free_window_info_fields, window_info_to_c}; -use crate::error::{clear_last_error, set_last_error, AdResult}; +use crate::error::{set_last_error, AdResult}; use crate::ffi_try::{trap_panic, trap_panic_void}; use crate::types::AdWindowInfo; use crate::AdAdapter; @@ -27,7 +27,6 @@ pub unsafe extern "C" fn ad_list_windows( }; match adapter.inner.list_windows(&filter) { Ok(windows) => { - clear_last_error(); let c_wins: Vec = windows.iter().map(window_info_to_c).collect(); let count = c_wins.len() as u32; if c_wins.is_empty() { diff --git a/crates/ffi/src/windows/op.rs b/crates/ffi/src/windows/op.rs index 4578b8e..5a19837 100644 --- a/crates/ffi/src/windows/op.rs +++ b/crates/ffi/src/windows/op.rs @@ -1,4 +1,4 @@ -use crate::error::{clear_last_error, set_last_error, AdResult}; +use crate::error::{set_last_error, AdResult}; use crate::ffi_try::trap_panic; use crate::types::{AdWindowInfo, AdWindowOp, AdWindowOpKind}; use crate::windows::to_core::ad_window_to_core; @@ -27,10 +27,7 @@ pub unsafe extern "C" fn ad_window_op( AdWindowOpKind::Restore => WindowOp::Restore, }; match adapter.inner.window_op(&core_win, core_op) { - Ok(()) => { - clear_last_error(); - AdResult::Ok - } + Ok(()) => AdResult::Ok, Err(e) => { set_last_error(&e); crate::error::last_error_code() diff --git a/crates/ffi/tests/error_lifetime.rs b/crates/ffi/tests/error_lifetime.rs new file mode 100644 index 0000000..2e6402e --- /dev/null +++ b/crates/ffi/tests/error_lifetime.rs @@ -0,0 +1,46 @@ +use agent_desktop_ffi::error::AdResult; +use std::ffi::CStr; + +#[allow(improper_ctypes)] +extern "C" { + fn ad_adapter_create() -> *mut agent_desktop_ffi::AdAdapter; + fn ad_adapter_destroy(adapter: *mut agent_desktop_ffi::AdAdapter); + fn ad_launch_app( + adapter: *const agent_desktop_ffi::AdAdapter, + id: *const std::os::raw::c_char, + timeout_ms: u64, + out: *mut agent_desktop_ffi::AdWindowInfo, + ) -> AdResult; + fn ad_last_error_message() -> *const std::os::raw::c_char; + fn ad_last_error_code() -> AdResult; + fn ad_check_permissions(adapter: *const agent_desktop_ffi::AdAdapter) -> AdResult; +} + +#[test] +fn last_error_pointer_survives_across_successful_calls() { + unsafe { + let adapter = ad_adapter_create(); + assert!(!adapter.is_null()); + + let bad_id = std::ptr::null(); + let mut out_win: agent_desktop_ffi::AdWindowInfo = std::mem::zeroed(); + let rc = ad_launch_app(adapter, bad_id, 0, &mut out_win); + assert_eq!(rc, AdResult::ErrInvalidArgs); + + let first_msg_ptr = ad_last_error_message(); + assert!(!first_msg_ptr.is_null()); + let first_msg = CStr::from_ptr(first_msg_ptr).to_string_lossy().into_owned(); + + for _ in 0..10 { + let _rc = ad_check_permissions(adapter); + } + + let later_msg_ptr = ad_last_error_message(); + assert_eq!(first_msg_ptr, later_msg_ptr); + let later_msg = CStr::from_ptr(later_msg_ptr).to_string_lossy().into_owned(); + assert_eq!(first_msg, later_msg); + assert_eq!(ad_last_error_code(), AdResult::ErrInvalidArgs); + + ad_adapter_destroy(adapter); + } +}