Init std.io.BufferedWriter outside main function avoid flushing

[tiawl]

Zig Version

0.13.0

Steps to Reproduce

Here a minimal exemple:

const std = @import ("std");

const EmptyMe = struct {
  const size = 4096;

  array: std.BoundedArray (u8, 2),
  buffered_stderr: std.io.BufferedWriter (size, std.fs.File.Writer) = undefined,
  stderr: std.io.BufferedWriter (size, std.fs.File.Writer).Writer = undefined,

  // Comment this and uncomment the matching function to make it works:
  fn init (writer: *const std.fs.File.Writer) !@This () {
    var self: @This () = .{
      .array = try std.BoundedArray (u8, 2).init (0),
      .buffered_stderr = std.io.bufferedWriter (writer.*),
    };
    self.stderr = self.buffered_stderr.writer ();
    return self;
  }

  // fn init () !@This () {
  //   return .{ .array = try std.BoundedArray (u8, 2).init (0), };
  // }

  fn empty (self: *@This ()) !void {
    defer {
      self.stderr.writeByte ('\n') catch {};
      self.buffered_stderr.flush () catch {};
    }
    while (self.array.len > 0) try self.stderr.writeByte (self.array.pop ());
  }
};

test "test" {
  const writer = std.io.getStdErr ().writer ();

  // Comment this and uncomment the comments below to make it works:
  var empty_me = try EmptyMe.init (&writer);

  // var empty_me = try EmptyMe.init ();
  // empty_me.buffered_stderr = std.io.bufferedWriter (writer);
  // empty_me.stderr = empty_me.buffered_stderr.writer ();

  try empty_me.array.append ('A');
  try empty_me.array.append ('B');
  try empty_me.empty ();
}

Observed Behavior

$ zig test test.zig
All 1 tests passed.

Expected Behavior

$ zig test test.zig
BA
All 1 tests passed.

Thank you for this awesome programming language.

[mlugg]

Your assignment to self.stderr in init takes a pointer to self.buffered_stderr, which is leaked out of the function by return self. Since &self.buffered_stderr is a pointer to a local variable, that means the stderr field becomes invalid. However, it is later used, by empty. That means this code exhibits Unchecked Illegal Behavior.

You could solve this by having init take an out-pointer rather than returning a value. One day, this use case might also be solved by Result Location Semantics combined with Pinned Structs. More simply, you could just not store buffered_stderr.writer(), and construct it as a local in empty -- it's very cheap to construct.

---

In future, please zig fmt your code before posting an issue. The code you posted is quite difficult to read for some of us who are very familiar with canonical Zig style.

Also, a few code style points:

• Defaulting those fields to undefined is a footgun. See the corresponding langref section for advice here.
• @This() is unnecessary here; just write EmptyMe.
• In init, that error should be handled with catch unreachable rather than try, because you know the error.Overflow case can't happen (since 0 is less than 2!).
• empty uses defer incorrectly. defer exists primarily for "cleanup" operations, which need to be run even in error cases etc. The code in your defer there should only run in the non-error case; if writing to stderr has already failed, you don't want to try to write more and try to flush. Moreover, using a defer here forces you to use catch {}, which explicitly hides potential errors. Instead, you should just try these lines after the loop.