How do I make this code using the `Writer` interface work?

Help
,
1

I’m trying to use the Writer interface to store the trace logs of the CPU execution of my emulator to a file, but it’s not working. For some reason, I’m seeing the output in the stdout and then at some point I get this error:

thread 33427 panic: Flush failed: error.WriteFailed

/home/labatata/Downloads/zig-x86_64-linux-0.15.2/lib/std/posix.zig:1551:22: 0x126ba76 in pwritev (ness)
            .BADF => return error.NotOpenForWriting, // Can be a race condition.
                     ^
/home/labatata/Downloads/zig-x86_64-linux-0.15.2/lib/std/fs/File.zig:1762:25: 0x126410a in drain (ness)
                        return error.WriteFailed;
                        ^
/home/labatata/Downloads/zig-x86_64-linux-0.15.2/lib/std/Io/Writer.zig:316:28: 0x11ccdba in defaultFlush (ness)
    while (w.end != 0) _ = try drainFn(w, &.{""}, 1);
                           ^
/home/labatata/Downloads/zig-x86_64-linux-0.15.2/lib/std/Io/Writer.zig:310:5: 0x11e71ba in flush (ness)
    return w.vtable.flush(w);
    ^
/home/labatata/Code/Zig/8bit-emulator/src/system.zig:182:51: 0x125d97a in dump_trace (ness)
        writer.flush() catch |err| std.debug.panic("Flush failed: {any}\n", .{err});
                                                  ^
/home/labatata/Code/Zig/8bit-emulator/src/system.zig:173:32: 0x1256003 in is_frame_complete (ness)
                self.dump_trace();
                               ^
/home/labatata/Code/Zig/8bit-emulator/src/system.zig:139:39: 0x124b67c in run_frame (ness)
        while (!self.is_frame_complete()) {
                                      ^
/home/labatata/Code/Zig/8bit-emulator/src/main.zig:105:25: 0x124a695 in main (ness)
        system.run_frame();
                        ^
/home/labatata/Downloads/zig-x86_64-linux-0.15.2/lib/std/start.zig:627:37: 0x124bd95 in main (ness)
            const result = root.main() catch |err| {

Here’s how I’m setting up the code

        var trace_file: ?std.fs.File = null;
        var trace_file_writer: ?std.Io.Writer = null;
        var trace_file_buffer: [4096]u8 = undefined;
        if (settings.trace_cpu) {
            trace_file = try std.fs.cwd().createFile("cpu.trace", .{});
            const w = trace_file.?.writer(&trace_file_buffer);
            trace_file_writer = w.interface;
        }

Then I keep tracing while the frame isn’t complete

    pub fn run_frame(self: *Self) void {
        while (!self.is_frame_complete()) {
            if (self.settings.trace_cpu) {
                trace(&self.trace_file_writer.?, self.cpu) catch @panic("Failed to trace CPU");
            }
            self.tick();
        }
    }

When it’s complete, I dump to the file

    pub fn is_frame_complete(self: *Self) bool {
        if (self.ppu.frame_complete) {
            self.ppu.frame_complete = false;
            if (self.settings.trace_cpu) {
                self.dump_trace();
            }
            return true;
        }
        return false;
    }

    fn dump_trace(self: *const Self) void {
        var writer = self.trace_file_writer.?;
        writer.flush() catch |err| std.debug.panic("Flush failed: {any}\n", .{err});
    }

Here’s the file that implements the trace function.

What I’m doing wrong here?

2

don’t copy interface, It operates on the assumption that it is used through a pointer to a field of the implementation type, it does pointer magic based on that assumption. If you do as you have done, you create undefined behaviour and are lucky if it breaks your code soon after. Otherwise, it could go unnoticed until it causes real damage.

My preferred way to prevent this, is to never store the interface in a variable, but to always access it through the implementation. (Excluding once it’s passed to something else ofc)

You have another issue, variables aren’t supposed to live beyond their scope, while it currently works if you keep it within the function (this will change in the future), once you leave the function it will once again cause undefined behaviour.
So you shouldn’t do the band-aid solution of making trace_file_writer a pointer to the interface.

Either you store the file writer implementation in your type, or you accept a writer from whoever instantiates your type.

3 Likes
3

Storing the writer implementation worked. But I have another problem, how do I make it work with the buffer?

Currently, I’m doing this

        var trace_file: ?std.fs.File = null;
        var trace_file_writer: ?std.fs.File.Writer = null;
        const trace_file_buffer = try allocator.alloc(u8, 4096);
        if (settings.trace_cpu) {
            trace_file = try std.fs.cwd().createFile("cpu.trace", .{});
            trace_file_writer = .initStreaming(trace_file.?, &trace_file_buffer);
        }

        return .{
             // other stuff...
            .trace_file = trace_file,
            .trace_file_writer = trace_file_writer,
            .trace_file_buffer = trace_file_buffer,
        };

But I get no content in the cpu.trace file when using the buffer. I only get the trace if I use no buffer &.{}. And, if I use .init instead of .initStreaming I only get the last line of the trace, why is that?

4

It sounds like you’re not flushing the writer.

The writer will only write the buffer contents by itself when the buffer fills up. This is to avoid expensive IO/syscalls. You need to call flush to write anything left in the buffer.

5

I’m calling flush on the dump_trace function

    fn dump_trace(self: *const Self) void {
        var writer = self.trace_file_writer.?;
        writer.interface.flush() catch |err| std.debug.panic("Flush failed: {any}\n", .{err});
    }