How to return a struct with slices

[solemn atlas]

Sorry, noob question. I have a simple struct

const KV = struct {
    key: []const u8,
    val: []const u8
};

A function is deserializing a []u8 into it. When I print it out inside the function all the data looks great, then outside, no bueno. Deserializing function. Pardon if the aloc makes legit no sense, I was just trying stuff to get it to work...

    fn deserializeCaskEntry(aloc: *const mem.Allocator, cask_bytes: []u8) !*KV {

        var kv = try aloc.create(KV);

        std.debug.print("Deserializing slice: {any}\n", .{cask_bytes});
        const size_len = @sizeOf(usize);
        const key_size = std.mem.bytesToValue(usize, cask_bytes[0..size_len]);
        const val_size = std.mem.bytesToValue(usize, cask_bytes[size_len..size_len*2]);
        std.debug.print("Got key size [{}] and val size [{}]\n", .{key_size, val_size});
        const key_bytes = cask_bytes[size_len*2..size_len*2 + key_size];
        const val_bytes = cask_bytes[size_len*2 + key_size..size_len*2 + key_size + val_size];

        kv.* = KV{.key = key_bytes, .val = val_bytes};

        std.debug.print("{any}\n", .{kv});
        return kv;
    }

This is called in

pub fn get(key: []const u8) !*KV {
        ... <setting up seek and printing stuff>
        _ = try current_file.read(bytes);
        const kv = deserializeCaskEntry(allocator, cask_bytes) catch |err| {
            std.debug.print("Encountered unknown error while deserializing bitcask: {any}\n", .{err});
            return BitcaskFileError.Unknown;
        };

        return kv;
    }

And this get is being called in a test in the file

test "Bitcask spec implementation: get" {

    const bc = BitCask;
    try bc.open("data");
    defer bc.close();

    try bc.put(.{.key = "2", .val = "secret"});
    const kv = try bc.get("2");
    std.debug.print("Got kv back\nKey: {any}\tVal: {any}", .{kv.key, kv.val});
}

[agile palm]

replace this line

// kv = &KV{.key = key_bytes, .val = val_bytes};
kv.* = KV{.key = key_bytes, .val = val_bytes};

the issue is that you're re-assigning the pointer (kv is a *KV) to point to a stack local.

instead you need to follow the pointer and assign to the KV value it points at.

this is a common mistake in zig and c. unfortunately, zig doesn't have safeguards to prevent this. there are proposals to it.

[solemn atlas]

Makes total sense

[agile palm]

*prevent it from happening.

[solemn atlas]

Replaced that line, but I'm still getting this *printing KV output

bitcask.KV{ .key = { 50 }, .val = { 115, 101, 99, 114, 101, 116 } }
<Inside function ^^>
Got kv back
Key: { 170 }    Val: { 170, 170, 170, 170, 170, 170 }

[agile palm]

i'm kindof surprised the compiler accepted it actually. i would expect &KV{...} to be of type *const KV

i don't think its the problem, but, you don't need to pass your allocator as *const mem.Allocator. it can just be mem.Allocator

[solemn atlas]

Maybe I should add more context on the whole process, I'm deserializing in a pub fn get(), then returning that kv to a zig test, which is getting the 170 values instead of what they are supposed to be.

[solemn atlas]

Not the problem, but it still compiles!

[agile palm]

think you have made a similar mistake. 170, 170 ... is 0xaa, 0xaa which means undefined. in debug builds, zig sets undefined things to this value

[solemn atlas]

Thanks for the tip, and as an aside, is the process of using aloc.create() like I did correct? I'm just reading the std docs and taking my best guess...

[solemn atlas]

oh that's very good to know

[agile palm]

yeah the way you're going alloc.create() looks fine to me. would need to see more code i think.

[solemn atlas]

Hmm alright, good idea, I'll edit the main post, one sec...

[agile palm]

are you perhaps deallocating the byte slice cask_bytes before its being printed?

no, i don't think that makes sense actually. would segfault in that case.

[solemn atlas]

Updated

I hit the limit so I trimmed some, lmk if anything doesn't make sense, thanks for taking the time to help 🙂

[agile palm]

what is the lifetime of cask_bytes ? it needs to outlive the get() method.

[solemn atlas]

Definitely doesn't, sits inside get()

[agile palm]

if its being read from a file, you could either use the allocator.dupe() on that slice or perhaps use a different read method. i think file has readAllAlloc() .

[solemn atlas]

var cask_bytes: []u8 = buffer[0..cask.size];
_ = try current_file.read(cask_bytes);

Right above the lines I started with in get

Then those are passed into deserialize

[agile palm]

and what is buffer?

[solemn atlas]

Right above that:

try current_file.seekTo(offset);
var buffer = try allocator.alloc(u8, cask.size);

Sorry, you are getting it in reverse order..

[agile palm]

oh i see. are you actually reading from the file into buffer? that seems like it might be the case.

[solemn atlas]

yes

[agile palm]

hmm. ok well its definitely some type of lifetime issue. its still tough to say whats happening.

[solemn atlas]

Reading into buffer, putting into slice of u8, then deserializing that slice into struct

[agile palm]

maybe show more of the get() method

[solemn atlas]

Well I can keep playing around with it, you've been super helpful, I can post again when I make some more progress printing stuff out.

[agile palm]

ok will keep an eye on it. good luck!

[solemn atlas]

Full Get

    pub fn get(key: []const u8) !*KV {
        std.debug.print("Getting value with key {s}\n", .{key});
        const optional_cask = keyDir.get(key);
        if (optional_cask == null) {
            return error.NoCaskFound;
        }
        const cask = optional_cask.?; // shorthand for 'optional_cask orelse unreachable'
        std.debug.print("Got cask from file {s}\n", .{cask.file_name});
        std.debug.print("Current position: {any}\n", .{current_file.getPos()});
        const offset = @intCast(u64, cask.offset);
        std.debug.print("Seeking to {}\n", .{offset});
        try current_file.seekTo(offset);
        var buffer = allocator.alloc(u8, cask.size) catch |err| {
            std.debug.print("Encountered unexpected error allocating buffer: {}\n", .{err});
            return error.Unknown;
        };
        defer allocator.free(buffer);
        var cask_bytes: []u8 = buffer[0..cask.size];
        _ = current_file.read(cask_bytes) catch |err| {
            std.debug.print("Encountered unexpected error allocating buffer: {}\n", .{err});
            return error.Unknown;
        };
        const kv = deserializeCaskEntry(allocator, cask_bytes) catch |err| {
            std.debug.print("Encountered unknown error while deserializing bitcask: {any}\n", .{err});
            return BitcaskFileError.Unknown;
        };

        return kv;
        // TODO get entry from db rather than return the keyDir entry
    }

omg

i just got it

[agile palm]

btw, you can use ```ts to get SOME highlighting. or rs works too. unfortunately zig doesn't work

[solemn atlas]

commented out the defer allocator.free(buffer)

[agile palm]

i see it. yep thats what i was going to say too 😄

[solemn atlas]

So then, my understanding lacks, when does that get freed? Or how do you manage that?

That seems like a memory leak to me... alloc something in a function and dont free it as the function exits?

[agile palm]

not sure if its the best, but you could return the whole buffer too so that it can be freed outside the function.

ie

pub fn get(key: []const u8) !struct{*KV, []u8} {

[solemn atlas]

oh interesting...

then just free it once the data has been passed off to where it needs to go...

[agile palm]

yes. something like that. btw, this is how you can declare a tuple in case you haven't seen it before.

[solemn atlas]

Awesome, thanks for the insight! Really appreciate it!

[agile palm]

fields a re unnamed. but you can access with result[0], result[1]

and to return one, just do

return .{kv, buffer};

[solemn atlas]

Thanks!

[agile palm]

np. let me know if you have any other questions.

[gusty slate]

Your intuition is correct that it's a leak -- but that's what you want in part; freeing it makes no sense because your caller wants it, so by definition you have to 'leak' it.
BUT, it's only actually a leak if you never free it.

If your program is short-lived, or otherwise soon-to-exit, then it's fine to leak it; the OS will reclaim it all anyway when you quit. But otherwise, you do probably want to free it at some point. 😄

So, once you've returned it, it'd be up to the caller to choose when (or if) to free it; it's their responsibility to manage the lifetime as they please.

If they only need it until they return, then they might defer allocator.free(the_answer); - or maybe they made an arena, and passed that to you - in which case they can just do defer arena.deinit();, etc, etc.

However, Travis makes a good point - you can only free the exact thing that you got back from the allocator -- you cannot do this:

const buf = try allocator.alloc(u8, 32);
const part = buf[16..];
defer allocator.free(part);

So, you'd need a way to retrieve the original buffer.

For example - If I'm reading this right, you could---if you wanted to---recreate it from the key_bytes; you always make key_bytes start at index 16 into that buffer, which means that something along these lines would work:

const original = (kv.key_bytes.ptr - 16)[0..cask.size];
allocator.free(original); // fine

...but it's generally better to not be clever -- especially given that this doesn't check that you've got it right in any way.
So you could just return the entire buffer as Travis suggests. 😄