From f31f8bb78236a8388fc7af9f5621639ca812f879 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 19 Mar 2025 20:55:41 -0700 Subject: [PATCH] apprt/embedded: utf8 encoding buffer lifetime must extend beyond call Fixes #6821 UTF8 translation using KeymapDarwin requires a buffer and the buffer was stack allocated in the coreKeyEvent call and returned from the function. We need the buffer to live longer than this. Long term, we're removing KeymapDarwin (there is a whole TODO comment in there about how to do it), but this fixes a real problem today. --- src/Surface.zig | 2 +- src/apprt/embedded.zig | 18 ++++++++++++++++-- src/build/Config.zig | 2 +- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index c96c92db1..46fa476f7 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -1727,7 +1727,7 @@ pub fn keyCallback( self: *Surface, event: input.KeyEvent, ) !InputEffect { - // log.debug("text keyCallback event={}", .{event}); + // log.warn("text keyCallback event={}", .{event}); // Crash metadata in case we crash in here crash.sentry.thread_state = self.crashThreadState(); diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index 4ad0390df..9ae00ab8e 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -149,8 +149,17 @@ pub const App = struct { } /// Convert a C key event into a Zig key event. + /// + /// The buffer is needed for possibly storing translated UTF-8 text. + /// This buffer may (or may not) be referenced by the resulting KeyEvent + /// so it should be valid for the lifetime of the KeyEvent. + /// + /// The size of the buffer doesn't need to be large, we always + /// used to hardcode 128 bytes and never ran into issues. If it isn't + /// large enough an error will be returned. fn coreKeyEvent( self: *App, + buf: []u8, target: KeyTarget, event: KeyEvent, ) !?input.KeyEvent { @@ -217,7 +226,6 @@ pub const App = struct { // Translate our key using the keymap for our localized keyboard layout. // We only translate for keydown events. Otherwise, we only care about // the raw keycode. - var buf: [128]u8 = undefined; const result: input.Keymap.Translation = if (is_down) translate: { // If the event provided us with text, then we use this as a result // and do not do manual translation. @@ -226,7 +234,7 @@ pub const App = struct { .composing = event.composing, .mods = translate_mods, } else try self.keymap.translate( - &buf, + buf, switch (target) { .app => &self.keymap_state, .surface => |surface| &surface.keymap_state, @@ -360,7 +368,9 @@ pub const App = struct { event: KeyEvent, ) !bool { // Convert our C key event into a Zig one. + var buf: [128]u8 = undefined; const input_event: input.KeyEvent = (try self.coreKeyEvent( + &buf, target, event, )) orelse return false; @@ -1432,7 +1442,9 @@ pub const CAPI = struct { app: *App, event: KeyEvent, ) bool { + var buf: [128]u8 = undefined; const core_event = app.coreKeyEvent( + &buf, .app, event.keyEvent(), ) catch |err| { @@ -1684,7 +1696,9 @@ pub const CAPI = struct { surface: *Surface, event: KeyEvent, ) bool { + var buf: [128]u8 = undefined; const core_event = surface.app.coreKeyEvent( + &buf, // Note: this "app" target here looks like a bug, but it is // intentional. coreKeyEvent uses the target only as a way to // trigger preedit callbacks for keymap translation and we don't diff --git a/src/build/Config.zig b/src/build/Config.zig index e0c7ed2be..48456734a 100644 --- a/src/build/Config.zig +++ b/src/build/Config.zig @@ -72,7 +72,7 @@ pub fn init(b: *std.Build) !Config { if (result.result.os.tag == .macos and builtin.target.os.tag.isDarwin()) { - result = genericMacOSTarget(b, null); + result = genericMacOSTarget(b, result.query.cpu_arch); } // If we have no minimum OS version, we set the default based on