Reader return EndOfStream before remaining bytes

Explain
1

Today’s rabbit hole comes from《Advent of Zig 2015年12日》, in which I’m forced to read a file against my will. Thus, I read.

The Reader

test {
    const io = std.testing.io;
    const file = try std.Io.Dir.cwd().openFile(io, "15-12", .{});
    defer file.close(io);

    var buff: [64]u8 = undefined;
    var reader = file.reader(io, &buff);
    const r = &reader.interface;

    log("\n{any}\n", .{file.length(io)});

    ...

}

Pretty standard as far as I understand it. Although, the absolutely necessary & in const r = &reader.interface; was not immediately obvious for a Zig n00b such as myself.

The Reading

test {

    ...

    var n: u32 = 0;
    while (true) : (n += 1) {
        const slice = r.take(buff.len) catch break;

        ....

    }
}

My first approach was something like this.

But “this” doesn’t read the entire thing. When reaching the end of the file I get a EndOfStream and break through the while loop. This is due to Reader > take() > peek() > fill() > fillUnbuffered() > try r.vtable.readVec > etc the point being that the mayority of this functions state:

If there are fewer than n bytes left in the stream, error.EndOfStream is returned instead.

The Journey

My first instinct was to just look for a function that wouldn’t do such thing, in the end, there are lots of them that return some kind of []u8 so it’s just a matter of time until I find one such as the returning slice has a length equal to the remaining characters to be read.

:clown_face: To my surprise, after reading the entirety of the Reader.zig file I couldn’t find a function that does this in a easy-enough way for me to understand.
For example:

  • streamRemaining requires a writer for which I can’t justify its creation.
  • allocRemaining (and friends) require an allocator that I don’t need, given my buffer.

At some point I found a cute little function called bufferedLen and I really liked it’s r.end - r.seek pattern. Curiously enough, the same subtraction is done by hand in many places in the code instead of using this function, which I found curious. I was expecting it to be inlined too, or to have a private inlined version. So I came back to my code and did:

test {

    ...

    var n: u32 = 0;
    while (true) : (n += 1) {
        const slice = r.take(buff.len) catch try r.take(r.end - r.seek);

        ....

    }
}

Problem solved. But.

Questions

What the hell?

This was a lot of work for such a stupid resolution and I’m almost 100% that I’m missing something really obvious. If Andrew responds again with a single builtin function name I’m gonna scream.

Some questions for you!

  • How would you write this?
  • Is the idea to peek() and then take()/toss() after handling errors?
  • Why isn’t there a function that returns a ?[]u8 so I can just plonk it into a while loop?

I you read all this, you are a champ.

2

I’m not really sure about what you actually want or need as you haven’t really talked about what you tried to achieve, but here’s my guess nonetheless: you want to read a file line by line?
If that’s the case you could take a look at peekDelimiter and friends. And also this post here.

If you really want to read it in segments of size buff.len you could take a look at readSliceShort or readSliceAll if you do want the error when finished.

Also I would personally just do this here:

var reader = file.reader(io, &buff);
// ...
const slice = try reader.interface.take(buff.len) catch break;

Instead of const r = &reader.interface. It’s a bit more typing but much clearer and you don’t have to come up with two names for (essentially) the same thing.

3

The idea is they do exactly what you ask of them take(5) requests 5 bytes.

(5 being a placeholder for your buffer length)
Your issue is the amount of data in the file is not a multiple of 5, so the last take find less than 5 bytes, that is not what you asked for so that is an error.

I would write something like this

while (true) : (n += 1) {
    try r.fillMore(); //read some amount of data
    const slice = r.buffered(); // get that data
    r.tossBuffered(); // tell reader that data has been read

    // alternatively
    // the difference being this will always try to fill the whole buffer
    // whereas the former
    const slice = r.take(buf.len) catch |e| if (e == error.EndOfStream) slice: {
        const s = r.buffered();
        r.tossBuffered();
        break :slice s;
    } else return e;
}

This also works, and may be preffered!
readSliceShort is the semantics op wants, but be aware read functions write to an out buffer and that should not be the interfaces buffer.

If you are not using the interfaces buffer elsewhere then you can disable it by giving the interface a 0 length buffer.

IDK, zig is wip, it may just not have occurred to anyone, the api is largely what was needed/nice for the compiler and its tools.
But it is also possible there is a reason it was explicitly omitted.

In any case you can implement such a function yourself :3

4

If you have a File.Reader, you can check the seek position, compare it to the file length and take the buffer size or the remaining bytes, depending on which is the smallest. This would avoid the error at the last read.

const file = try std.Io.Dir.cwd().openFile(io, sub_path, .{});
defer file.close(io);

var buffer: [64]u8 = undefined;
var file_reader = file.reader(io, &buffer);

const file_len = try file.length(io);

while ((file_len - file_reader.pos) > 0) {
    std.log.debug("Reading {d} bytes from {d} to {d} (remaining bytes: {d})", .{
        @min(buffer.len, file_len - file_reader.pos),
        file_reader.pos,
        file_reader.pos + @min(buffer.len, file_len - file_reader.pos),
        file_len - file_reader.pos,
    });
    const bytes = try file_reader.interface.take(@min(buffer.len, file_len - file_reader.pos));
    _ = bytes; // Do something with bytes
}

This outputs:

…
debug: Reading 64 bytes from 8384 to 8448 (remaining bytes: 83)
debug: Reading 19 bytes from 8448 to 8467 (remaining bytes: 19)

5

This is neat to demonstrate the issue

but I have to point out that no one should use this as a solution.

6

You do not need to read more than one character at a time. takeByte() solves this puzzle without much trouble:

while (reader.interface.takeByte()) |ch| {
  switch (state) {
    .container => switch (ch) {
      '\"' => state = .string,
      // ...
    },
    .string => switch (ch) {
      '\"' => state = .container,
      'a'...'z' => {},
      else => return error.InvalidCharacter,
    },
    // ...
  }
} else |err|  switch (err) {
  error.EndOfStream => {},
  error.ReadFailed => return reader.err.?,
}

… will soon run into error.StreamTooLong in this puzzle. No fun, not even worth it.

3 Likes
7

What would be the issue with using this as a solution? I’m still learning Zig so I’d be happy to hear why.

8

It unnecessarily relies not only on there being a file, but for it to be seekable.

It is just far less portable/reusable than it could be with no added benefit and its not less work/code either.

This is the best answer! I should have looked at the puzzle.

3 Likes
9

Thanks for your answer! I honestly don’t know how I missed it, but readSliceShort is exactly what I was looking for. I don’t understand why the name is so weird given that this might be the single most useful function in the Reader, at least when dealing with complete reads of files.

I understand why you say this but I just don’t like to write such boilerplate each time, although for a real implementation I wouldn’t call it just r.

Thanks for your answer! Yes, the use of blocks and taking/peeking/discarding/tossing by hand was one of the many implementations I wrote, but as you say later in the post, ultimately I was looking for readSliceShort and not finding it.

OwO - I might, although I doubt they would accept such a change for a single person’s need. It would be very ergonomic tho and I might write it for myself as a drop-in for future projects.

Thanks for your answer! Yes, this was another of my implementations, but the same can be done just using the reader and leaving the file handling fight to Zig and whatever OS is running the thing.

Thanks for your answer! The thing is that my intention was not to solve it (thanks for the tip tho) but to understand better how the reader is implemented and discover any useful functions that are common knowledge that I still haven’t attained.

In any case, even for solving the puzzle, it pains my heart the unhinge amount of IO operations reading byte by byte must be performing :crying_cat:.

10

The reader interface contains a buffer to make that a non-issue, that is a big part why it was designed that way.

11

If you mean the one I’m providing, I don’t think it will be read into unless I ask for it.

If you mean an internal buffer, can you please point to it in the code? I’m only able to find the one I provide in /usr/lib/zig/std/Io/Reader.zig ln 17.

12

It will. It always try to fill it as much as possible.

1 Like
13

Oh yeah, just tested it and yup. I was mistaking the fill(r, 1) for filling just one, but it’s description is clear enough.

Fills the buffer such that it contains at least n bytes, without advancing the seek position.

Still, I can’t find the reader interface buffer, I can only find a slice to the buffer I provided. At this point I have no evidence of it reading into an internally contained buffer.

1 Like
14

It reads into that buffer that you provide for it.

1 Like
15

I think we are tail-spinning a bit.

When you initialize the reader, it asks a slice of you.
You can give it either empty slice for unbuffered reading.
Or if you provide it a slice to a buffer it will:

  • use this buffer for buffered reading, every time it runs empty, it will fill it as much as it can (depends on underlying implementations but usually 1 syscall) and then operate on it untill it runs empty again.
  • when you ask for peek functions it will do them inside the buffer (possibly filling it when it has some empty space), if it cant find it inside the buffer will return error indicating that it failed.
  • if you call take functions that return a slice (non-allocating ones) it will return slice pointing inside this buffer, again returning error if it fails because of the slice needed being bigger than buffer.
  • you overall should not touch this buffer, all the interactions should happen thru the interface and calling functions like fill should be avoided unless you are implementing specialized reading function operating directly on the buffer yourself. All the abstracted functions handle its own filling themselves.

This interface is a bit confusing for several reasons:

  • it does all of this without dynamic allocations
  • its an intrusive interface, that is the reason why you cant safely copy it and you allways take it as a pointer (retrieves its implementation data via @fieldParentPtr())
  • it has the buffer variable (the one you provide) inside of the interface, this allows it to trace function pointers only when it needs to flush (writer) or fill (reader) more. That way the optimizer sees all the operations on the buffer and produces a lot more efficient code.

Overall handeling arbitrary long strings without memory allocations is a bit messy, so either allocate and stream it to an std.Io.Writer.Allocating (which allows you to simply reuse the memory for several readings with clearRetainingCapacity()), or make sure your buffer is large enough to hold all the data you need at the same time (if you can process this arbitrary string piece by piece).

I personally often reach for takeByte() when doing input parsing of a format and if i have arbitrary long strings in it, then streaming them up to a delimiter to an std.Io.Writer.Allocating is still decently optimal and easy to use way (often desired if you need all of the string at the same time and cant process it piece by piece).

Hope it explained some stuff
Robert :blush:

5 Likes
16

Thanks for your answer!

Yeah that was the point xD

Yup, I think I’m gonna go for that since it aligns with the way I wanna solve the problem, and now I know it’s not doing IO each time.

I’m not a fan of allocating, but if I were to go for the second approach to solving this (search for “red” then search for it’s containing object or array and delete those) I would use the allocating thingy and just mem.window over the whole file.

17

Final solution. Instead of making nodes and the whole tree structure (already did a depth-first search exercise) I just search for "red", find the bounds for the container and delete it if it’s an object.

Ironically, I need to read the whole file at once, so this post was “for nothing” but learning how the reader works… I’m still pretty happy because it’s just the 12th exercise and I’m writing code soooo fast now comparatively.

I know I do extremely unsafe things here but since I know the input, I just don’t care. Any comments on my terrible code is welcome.

const std = @import("std");
const log = std.debug.print;
const tokenize = std.mem.tokenizeAny;

pub fn main(init: std.process.Init) !void {
    const io = init.io;

    const file = try std.Io.Dir.cwd().openFile(io, "15-12", .{});
    defer file.close(io);

    var buff: [1024 * 64]u8 = @splat(' ');
    var stream = file.readerStreaming(io, &buff);
    var reader = &stream.interface;

    var slice = buff[0..try reader.readSliceShort(&buff)];

    log("part one > {d}\n", .{try sum(slice)});
    sanitize(&slice);
    log("part one > {d}\n", .{try sum(slice)});
}

fn sum(slice: []u8) !i64 {
    var n: i64 = 0;

    var it = tokenize(u8, slice, " :;{}[],.+\"\'qwertyuiopñlkjhgfdsazxcvbnm");
    while (it.next()) |val|
        n += try std.fmt.parseInt(i64, val, 10);

    return n;
}

fn sanitize(slice: *[]u8) void {
    const view = slice.*;
    
    var window = std.mem.window(u8, view, 3, 1);
    while (window.next()) |val| {
        if (!std.mem.eql(u8, val, "red"))
            continue;

        var l: usize = window.index.? - 1;
        var r: usize = l + 2;

        containerBounds(view, &l, &r);

        if (view[l] == '{' and view[r] == '}')
            @memset(slice.*[l .. r + 1], ' ');
    }
}

fn containerBounds(slice: []u8, l: *usize, r: *usize) void {
    var lCount: usize = 1;
    var rCount: usize = 1;

    while (lCount > 0 or rCount > 0) {
        if (lCount > 0) {
            l.* -= 1;
            switch (slice[l.*]) {
                '{', '[' => lCount -= 1,
                '}', ']' => lCount += 1,
                else => continue,
            }
        }
        if (rCount > 0) {
            r.* += 1;
            switch (slice[r.*]) {
                '{', '[' => rCount += 1,
                '}', ']' => rCount -= 1,
                else => continue,
            }
        }
    }
}
18

This is not true, but I do think it makes it easier.

the interface buffer (passed to readerStreaming) and the out buffer for readSliceShort cannot be the same.

Given that you only use a single read* function, the interface buffer is unnecessary, you can just give it a 0 length buffer &.{}.

Also, reader can, and should, be const.

no reason to have a pointer to a slice, a slice is already a pointer (+ len); you are not modifying the slice itself, only the data within

1 Like
19

I would allocate it on heap at this point.

Zig source encoding is utf-8. The ñ letter is two bytes [2]u8{ 195, 177 } in utf-8. tokenize() expects single-byte encoding like ascii. I guess you just pressed every key on your keyboard without thinking about languages, alphabets and encodings. This will cause trouble eventually.

Why slop two loops together?

while (lCount > 0) {
  // ...
}
while (rCount > 0) {
  // ...
}
3 Likes
20
1 Like