From b5bd35f5387058e40903553bc152232b62172b0a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 20 Aug 2023 10:39:42 -0700 Subject: [PATCH] terminal/kitty-gfx: optimize some of our kv parsing --- src/terminal/kitty/graphics.zig | 102 ++++++++++++++++++++++++-------- 1 file changed, 77 insertions(+), 25 deletions(-) diff --git a/src/terminal/kitty/graphics.zig b/src/terminal/kitty/graphics.zig index 67ed72b9e..9f583fc5b 100644 --- a/src/terminal/kitty/graphics.zig +++ b/src/terminal/kitty/graphics.zig @@ -4,6 +4,7 @@ //! https://sw.kovidgoyal.net/kitty/graphics-protocol const std = @import("std"); +const assert = std.debug.assert; const Allocator = std.mem.Allocator; const ArenaAllocator = std.heap.ArenaAllocator; @@ -42,11 +43,12 @@ pub const CommandParser = struct { state: State = .control_key, const State = enum { - /// We're parsing the key of a KV pair. + /// Parsing k/v pairs. The "ignore" variants are in that state + /// but ignore any data because we know they're invalid. control_key, - - /// We're parsing the value of a KV pair. + control_key_ignore, control_value, + control_value_ignore, /// We're parsing the data blob. data, @@ -76,31 +78,36 @@ pub const CommandParser = struct { switch (self.state) { .control_key => switch (c) { // '=' means the key is complete and we're moving to the value. - '=' => { - // All keys at the time of writing this parser are one char - if (self.kv_temp_len != 1) @panic("TODO"); + '=' => if (self.kv_temp_len != 1) { + // All control keys are a single character right now so + // if we're not a single character just ignore follow-up + // data. + self.state = .control_value_ignore; + self.kv_temp_len = 0; + } else { self.kv_current = self.kv_temp[0]; self.kv_temp_len = 0; self.state = .control_value; }, - else => try self.accumulateValue(c), + else => try self.accumulateValue(c, .control_key_ignore), + }, + + .control_key_ignore => switch (c) { + '=' => self.state = .control_value_ignore, + else => {}, }, .control_value => switch (c) { - // ',' means we're moving to another kv - ',' => { - try self.finishValue(); - self.state = .control_key; - }, + ',' => try self.finishValue(.control_key), // move to next key + ';' => try self.finishValue(.data), // move to data + else => try self.accumulateValue(c, .control_value_ignore), + }, - // ';' means we're moving to the data - ';' => { - try self.finishValue(); - self.state = .data; - }, - - else => try self.accumulateValue(c), + .control_value_ignore => switch (c) { + ',' => self.state = .control_key_ignore, + ';' => self.state = .data, + else => {}, }, .data => try self.data.append(self.arena.allocator(), c), @@ -119,11 +126,12 @@ pub const CommandParser = struct { switch (self.state) { // We can't ever end in the control key state and be valid. // This means the command looked something like "a=1,b" - .control_key => return error.InvalidFormat, + .control_key, .control_key_ignore => return error.InvalidFormat, // Some commands (i.e. placements) end without extra data so // we end in the value state. i.e. "a=1,b=2" - .control_value => try self.finishValue(), + .control_value => try self.finishValue(.data), + .control_value_ignore => {}, // Most commands end in data, i.e. "a=1,b=2;1234" .data => {}, @@ -172,15 +180,23 @@ pub const CommandParser = struct { }; } - fn accumulateValue(self: *CommandParser, c: u8) !void { - self.kv_temp[self.kv_temp_len] = c; + fn accumulateValue(self: *CommandParser, c: u8, overflow_state: State) !void { + const idx = self.kv_temp_len; self.kv_temp_len += 1; - if (self.kv_temp_len > self.kv_temp.len) @panic("TODO"); + if (self.kv_temp_len > self.kv_temp.len) { + self.state = overflow_state; + self.kv_temp_len = 0; + return; + } + self.kv_temp[idx] = c; } - fn finishValue(self: *CommandParser) !void { + fn finishValue(self: *CommandParser, next_state: State) !void { const alloc = self.arena.allocator(); + // We can move states right away, we don't use it. + self.state = next_state; + // Check for ASCII chars first if (self.kv_temp_len == 1) { const c = self.kv_temp[0]; @@ -819,3 +835,39 @@ test "delete command" { try testing.expectEqual(@as(u32, 3), dv.x); try testing.expectEqual(@as(u32, 4), dv.y); } + +test "ignore unknown keys (long)" { + const testing = std.testing; + const alloc = testing.allocator; + var p = CommandParser.init(alloc); + defer p.deinit(); + + const input = "f=24,s=10,v=20,hello=world"; + for (input) |c| try p.feed(c); + const command = try p.complete(alloc); + defer command.deinit(alloc); + + try testing.expect(command.control == .transmit); + const v = command.control.transmit; + try testing.expectEqual(Transmission.Format.rgb, v.format); + try testing.expectEqual(@as(u32, 10), v.width); + try testing.expectEqual(@as(u32, 20), v.height); +} + +test "ignore very long values" { + const testing = std.testing; + const alloc = testing.allocator; + var p = CommandParser.init(alloc); + defer p.deinit(); + + const input = "f=24,s=10,v=2000000000000000000000000000000000000000"; + for (input) |c| try p.feed(c); + const command = try p.complete(alloc); + defer command.deinit(alloc); + + try testing.expect(command.control == .transmit); + const v = command.control.transmit; + try testing.expectEqual(Transmission.Format.rgb, v.format); + try testing.expectEqual(@as(u32, 10), v.width); + try testing.expectEqual(@as(u32, 0), v.height); +}