Inconsistency between `Writer.[...]Preserve` functions

[rpkak]

I have some questions about what the Writer.[...]Preserve functions do. E.g.:

• Is asserted, that at least preserve_len bytes are already in the buffer?
• If the [...]Preserve function writes new content into the writer, should this new content be placed before or after the preserved bytes?
• If the [...]Preserve function writes new content into the writer, is the new content included in the preserve_len bytes or only bytes already in the buffer count?

The answer to be first question seems, at least for most [...]Preserve functions, to be, that the minimum of preserve_len and w.end is preserved.
But for some reason, which I don't understand, writableSlice(Greedy)Preserve asserts, that w.buffer.len >= preserve_len + minimum_length (EDIT: fixed in #24844).

The [...]Preserve functions do not agree in the other two questions:

writableSlice(Greedy)Preserve:

• new content after preserved bytes
• preserve_len does not include new content

EDIT: writePreserve and writeAllPreserve got deleted in #24790

writePreserve:

• order of preserved bytes and new content depends on unused buffer capacity and drain implementation
• preserve_len does not include new content

writeAllPreserve: (repeatedly calls writePreserve)

• Same as writePreserve
• repeatedly calls writePreserve, which means that in the second call of writePreserve the content of the first call to writePreserve is included in the preseved bytes, but I don't know if a drain implementation whould be valid if it would trigger this.

writeBytePreserve:

• new content after preserved bytes
• preserve_len does not include new content

I also found some cases, which seem like bugs:

• EDIT: writePreserve got deleted in Writer: Delete writePreserve/writeAllPreserve #24790

  As mentioned above:

  const std = @import("std");
  const assert = std.debug.assert;
  
  pub fn main() !void {
      {
          var file = try std.fs.cwd().createFile("1", .{});
          defer file.close();
          var buf: [3]u8 = undefined;
          var fb = file.writer(&buf);
          defer fb.interface.flush() catch unreachable;
  
          assert(try fb.interface.write("foo") == 3);
          assert(try fb.interface.writePreserve(3, "bar") == 3);
      }
      {
          var file = try std.fs.cwd().createFile("2", .{});
          defer file.close();
          var buf: [6]u8 = undefined;
          var fb = file.writer(&buf);
          defer fb.interface.flush() catch unreachable;
  
          assert(try fb.interface.write("foo") == 3);
          assert(try fb.interface.writePreserve(3, "bar") == 3);
      }
  }

  When executed the file 1 contains barfoo but the file 2 contains foobar.

• The function splatBytePreserve is not included in the list above, because I just don't understand, what it is should do:

  const std = @import("std");
  const assert = std.debug.assert;
  
  pub fn main() !void {
      var file = try std.fs.cwd().createFile("filename", .{});
      defer file.close();
      var buf: [40]u8 = undefined;
      var fb = file.writer(&buf);
      defer fb.interface.flush() catch unreachable;
  
      const content = "some content, which is 36 chars long";
  
      try fb.interface.writeAll(content);
      try fb.interface.splatBytePreserve(10, 'X', 10);
  }

  When executed the file filename contains chars longXXXXXXXXXX.

Probably related: #24700, #24741

[squeek502]

Worth noting that writeAllPreserve/writePreserve is currently fully unused/untested in the Zig codebase, so that could explain the inconsistent ordering going unnoticed.

splatBytePreserve is untested and has exactly 1 usage in zstd.Decompress.

Generally, seems like there's a need for a lot more testing/clarification around these APIs.

[andrewrk]

Generally, seems like there's a need for a lot more testing/clarification around these APIs.

I'm fairly certain it's going to need another breaking API change to add another index, similar to how the Reader buffer has both end and seek. This way instead of "hiding" data and restoring it, the pusher modifies only end and the puller modifies only seek.