strcat function

[rough belfry]

// ///
// /// concats str to buffer
// ///
pub fn strcat(allocator: std.mem.Allocator, dest: *[]u8, src: []const u8) !void {
    const oldsize = dest.len;
    const newsize = oldsize + src.len;
    dest.* = try allocator.realloc(dest.*, newsize);

    @memcpy(dest.*[oldsize..newsize], src);
}

test "strcat" {
    const allocator = std.testing.allocator;

    const str = "Hello";
    const str2 = " world!";
    var buffer = try allocator.alloc(u8, str.len);
    defer _ = allocator.free(buffer);
    @memcpy(buffer[0..str.len], str);

    _ = try strcat(allocator, &buffer, str2);

    try std.testing.expect(std.mem.eql(u8, buffer, "Hello world!"));
}

is it looks okay?

[still niche]

About the approach, why overload the dest parameter with both being the left operand for concatenation, and also the output buffer? It's a confusing interface. Also you are assuming that the caller will pass in a dynamically allocated dest pointer to buffer. This will not work if dest.* is stack allocated.

A better way would be to have an explicit out parameter. The caller would be responsible to make sure the buffer is large enough, and return error if not. As a bonus you would not need to an allocator.

fn strcat(left: []const u8, right: []const u8, out []u8) OutputTooBig!void

Or if you do want the function to handle the allocation for the user, then return the new buffer:

fn strcat(allocator: std.mem.Allocator, left: []const u8, right: []const u8) OutOfMemory![]u8

Another point is that *[]u8 is not necessary. It's essentially a double pointer. A slice already contains a pointer to memory and length.

[north kindle]

their use of *[]u8 is justified as they are reassigning where buffer is pointing to

[still niche]

Thanks I see now. Still an over complicated way of doing things.

[north kindle]

imo your function works as intended, but usually theres better ways of handing strings. need to append a lot? use ArrayList(u8), otherwise just use slices and try your best to avoid concatting and use a writer. this function is only good for appending once to a heap allocated string, which is pretty specific (once you append more than once you probably should have went with an arraylist)

[still niche]

And I think it would memory leak unless realloc happens to keep the same memory address (that's true at least for the c allocator)

[north kindle]

afaict theyre using it right. havent used realloc that much but from my testing this is ok

const a = std.testing.allocator;
for (0..100) |i| {
  var slice = try a.alloc(u8, 100);
  defer a.free(slice);
  slice = try a.realloc(slice, i * 100);
}

[north kindle]

only other thing id say is you dont have to discard void values, so you can get rid of both _ =s

[still niche]

My bad, I'm looking at the zig implementation now, which does free the old memory.

[rough belfry]

because the first one is heap allocated string

and i wanna add str into it

not just two strings

[chilly hearth]

There is also std.mem.concat

[rough belfry]

i didn't find any example of it