summaryrefslogtreecommitdiff
path: root/src/look.zig
diff options
context:
space:
mode:
authorGabriel Schneider <[email protected]>2026-09-03 15:39:43 -0300
committerGabriel Schneider <[email protected]>2026-09-03 15:39:43 -0300
commitd89c0b532df23ed5b48495f83725893d5d82042b (patch)
treea4a2eff66096fb90414bf5cd84fe016c930e0d68 /src/look.zig
parent3d8d4425c969d3df21915c9c14b144460a1c0086 (diff)
downloadpardes-d89c0b532df23ed5b48495f83725893d5d82042b.tar.gz
pardes-d89c0b532df23ed5b48495f83725893d5d82042b.zip
errors: a save that could not happen, and two panics on an ordinary click
A review of what this program does when the environment says no. The finding that reframes it: there were almost NO panics on ordinary paths — the rule already held — but there was a great deal of silence, and one case worse than any panic. SILENT DATA LOSS ON SAVE. `saveFile` marked the pane saved the moment it QUEUED the effect, before any host had tried; `host_io.writeFd` returned void, so a short or failed write was indistinguishable from a complete one; and `writeFileBytes` returned true regardless. A save to a read-only file, or into a directory removed under the pane, therefore cleared the tag's ` *` and posted nothing — and `Del` makes no dirty check, so the next click threw the edits away with the screen saying they were safe. On a full disk it was worse: the file is already `O_TRUNC`'d when `write` fails, so the message row said `saved` over a file that had just been emptied. Now: `writeFd` reports, `writeFileBytes` returns WHY (`PermissionDenied`, `NoSpaceLeft`, `ReadOnlyFilesystem`, …) including a failed `close`, which is where write-back filesystems report at all; the core marks the pane saved around `perform` rather than at emit, which is also where the bytes are read; and a host that could not write calls `Pardes.saveFailed`, which puts the reason on the message row and takes the clean mark back. That is a CALL and not a return value because host.zig enforces, at comptime, that a `push_` method reaching every host in a fan-out cannot have one answer — the first attempt at this changed the signature and the compiler was right to refuse it. TWO PANICS ON AN ORDINARY KEYSTROKE, in look.zig's number scans. `v = v * 10 + d` over caller-supplied digits, reached from `parsePathLine` and the `@pN` scan — which every Look, every right-click and every n/N motion runs on whatever word is under the pointer. A hash in a log, a CSV column, any output shaped `foo:99999999999999999999`, and the editor died with "integer overflow". Both saturate now, the same way acmefs.zig's address parser already did; a saturated line is refused by `file_pane.open`'s `line <= total` and a saturated pane id by `focusPaneLine`'s `id < MAX_PANES`, so nothing addressable changes. A BOOT FILE THAT WILL NOT OPEN joins the missing-name case in the `+Errors` pane instead of taking the launch down: `pardes /root` resolves as a `.file`, could not be read, and left `error: PermissionDenied` and a return trace. `look.readFile` now says which errno it was, so the pane can say "permission denied" rather than a word from the source code. The tag-marker test drained no effects and passed anyway, which is exactly the defect; it drains now. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_016Q4RATpafkwahrovHQLKRf
Diffstat (limited to 'src/look.zig')
-rw-r--r--src/look.zig60
1 files changed, 57 insertions, 3 deletions
diff --git a/src/look.zig b/src/look.zig
index 1730a456..f5d1488c 100644
--- a/src/look.zig
+++ b/src/look.zig
@@ -76,7 +76,17 @@ pub const Spot = struct {
fn num(tok: []const u8, i: usize) struct { v: usize, end: usize } {
var v: usize = 0;
var j = i;
- while (j < tok.len and std.ascii.isDigit(tok[j])) : (j += 1) v = v * 10 + (tok[j] - '0');
+ // SATURATING, and this is not defensive programming — it is the fix for a
+ // crash on an ordinary keystroke. The digits come off whatever word is
+ // under the pointer, so `*` and `+` here run on text the user never wrote
+ // and cannot control: a right-click, an Enter, or an `n` on anything shaped
+ // `foo:99999999999999999999` — a hash in a log, a column of a CSV, the
+ // output of any program — overflowed a `usize` and took the editor down
+ // with "integer overflow". A number too big to be a line is not a line, and
+ // `maxInt` is refused by every consumer for free: `file_pane.open` asks
+ // `line <= total` and `focusPaneLine` asks `id < MAX_PANES`. Same shape
+ // acmefs.zig's address parser already uses.
+ while (j < tok.len and std.ascii.isDigit(tok[j])) : (j += 1) v = v *| 10 +| (tok[j] - '0');
return .{ .v = v, .end = j };
}
@@ -175,6 +185,35 @@ test "parsePathLine: spots, ranges, and the paths that merely look like them" {
}
}
+test "a number too big to be a line saturates instead of taking the editor down" {
+ // These are keystrokes, not arguments. `parsePathLine` runs on whatever
+ // word is under the pointer on a right-click, an Enter or an `n` — so the
+ // digits come out of a hash in a log, a CSV column, or any program's
+ // output, and an unchecked `v * 10` there is a panic on ordinary use. Every
+ // number in the token comes through the same scan, so all four are tried.
+ const huge = "99999999999999999999999999";
+ const cases = [_][]const u8{
+ "f.zig:" ++ huge,
+ "f.zig:" ++ huge ++ ":" ++ huge,
+ "f.zig:1-" ++ huge,
+ "f.zig:1:2-" ++ huge ++ ":" ++ huge,
+ };
+ for (cases) |tok| {
+ const got = parsePathLine(tok);
+ try std.testing.expectEqualStrings("f.zig", got.path);
+ // Saturated rather than wrapped: a wrap would address a REAL line, and
+ // silently jumping somewhere is worse than not jumping.
+ try std.testing.expect(got.at.line >= 1);
+ }
+
+ // ...and the pane address, which has its own scan. `focusPaneLine` refuses
+ // anything past MAX_PANES, so this resolves to a pane that cannot exist.
+ var realbuf: [4096]u8 = undefined;
+ const target = resolve("@p" ++ huge, "/tmp", &realbuf);
+ try std.testing.expect(target == .pane);
+ try std.testing.expect(target.pane.id >= 16);
+}
+
test "parsePathLine: `end` separates a whole-token target from a lenient read" {
// the whole token IS the target: every spelling the doc above lists
for ([_][]const u8{
@@ -469,7 +508,10 @@ pub fn resolve(word_raw: []const u8, cwd: []const u8, realbuf: *[4096]u8) Target
var id: usize = 0;
for (word[config.pane_addr.len..]) |c| {
if (!std.ascii.isDigit(c)) break;
- id = id * 10 + (c - '0');
+ // Saturating for the same reason `num` above is: this scan also
+ // runs on a word somebody merely clicked. `focusPaneLine` refuses
+ // anything past `MAX_PANES`, so a saturated id addresses nothing.
+ id = id *| 10 +| (c - '0');
} else return .{ .pane = .{ .id = id, .at = pl.at } };
}
@@ -816,7 +858,19 @@ pub fn readFile(gpa: std.mem.Allocator, path: []const u8) ![]u8 {
var pathbuf: [4096]u8 = undefined;
const path_z = std.fmt.bufPrintSentinel(&pathbuf, "{s}", .{path}, 0) catch return error.PathTooLong;
const fd = libc.open(path_z, .{ .ACCMODE = .RDONLY });
- if (fd < 0) return error.OpenFailed;
+ // WHY it would not open, not just that it would not. Every one of these is
+ // an ordinary thing to do by accident — `pardes /root`, a file left at mode
+ // 000, a name that was deleted between resolving and reading — and a caller
+ // that can only say "OpenFailed" has to show the human a word that means
+ // nothing to them. `errno` is libc's here, which is the only reason it can
+ // be read off a `-1`: see the raw-syscall note in `termCwd`.
+ if (fd < 0) return switch (libc.errno(fd)) {
+ .ACCES, .PERM => error.PermissionDenied,
+ .NOENT => error.FileNotFound,
+ .ISDIR => error.IsDirectory,
+ .NAMETOOLONG => error.PathTooLong,
+ else => error.OpenFailed,
+ };
defer _ = libc.close(fd);
const end = libc.lseek(fd, 0, libc.SEEK.END);
const size: usize = if (end < 0) 0 else @intCast(end);