#Access on inner ArrayList of 2D Arraylist?

1 messages · Page 1 of 1 (latest)

blissful zodiac
#

Maybe a noobie question, but how can I append to the inner ArrayList of a 2D Arraylist?

pub fn function(nums: [3][3]i32) !void {
    var allocator_gpa = std.heap.GeneralPurposeAllocator(.{}){};
    defer _ = allocator_gpa.deinit();

    var diag = std.ArrayList(std.ArrayList(i32)).init(allocator_gpa.allocator());
    defer diag.deinit();

    for (nums, 0..) |row_val, row_idx| {
        for (row_val, 0..) |col_val, col_idx| {
            if (row_idx + col_idx < diag.items.len) {
                // problem is not a new row
                // but to append on the inner ArrayList
                std.debug.print("ADD TO EXISTING ROW \n", .{});
                std.debug.print("(row_idx, col_idx): ({d},{d}) \n", .{ row_idx, col_idx });
                std.debug.print("value: {d} \n", .{col_val});
                // fails: with memory leak
                try diag.items[row_idx + col_idx].append(col_val);
            } else {
                std.debug.print("NEW ROW \n", .{});
                std.debug.print("(row_idx, col_idx): ({d},{d}) \n", .{ row_idx, col_idx });
                std.debug.print("value: {d} \n", .{col_val});
                var new_row = std.ArrayList(i32).init(allocator_gpa.allocator());
                defer new_row.deinit();
                try new_row.append(col_val);
                std.debug.print("new_row: {any} \n", .{new_row.items});

                try diag.append(new_row);
            }
        }
    }
}
chrome fable
#

Also, you're doing new_row.deinit() and then adding that list to the list!

#

That is a use after free

#

You would need magic to save you there

#

Here there is none

#

You're just destroying the thing which the thing you're appending, relies on

#
        /// Release all allocated memory.
        pub fn deinit(self: Self) void {
            if (@sizeOf(T) > 0) {
                self.allocator.free(self.allocatedSlice());
            }
        }
        pub fn allocatedSlice(self: Self) Slice {
            // `items.len` is the length, not the capacity.
            return self.items.ptr[0..self.capacity];
        }
#

(From lib/std/array_list.zig)

blissful zodiac
#

Thx for the quick response.
I don't have a zig or c background, so maybe I'm mission some knowledge or understanding there.

So there are 2 problems:

  1. the wrong use of .deinit(). I was thinking that defere new_row.deinit() would handle it correctly.
    But it doesn't, after going out of scope in the else{...}
  2. and by using .items[i] getting a copy instead

Is this correct?

chrome fable
#

And yes, defer is just cutnpaste a statement or block to the end of the scope

#

[Edited]

blissful zodiac
#

Just for the background info, what I'm trying to do: Learn zig a little bit by solving some examples/leetcode like Diagonal Traverse II - LeetCode

I'm still struggling with it a bit (understanding zig and its syntax): So I go it running somehow, but it is not beautiful:

pub fn diagonalTraverseII_bucket(nums: [3][3]i32) !std.ArrayList(i32) {
    var allocator_gpa = std.heap.GeneralPurposeAllocator(.{}){};
    defer std.debug.print("GPA results: {}\n", .{allocator_gpa.deinit()});
    var logging_alloc = std.heap.loggingAllocator(allocator_gpa.allocator());
    const allocator = logging_alloc.allocator();

    var counter_diag_tier = std.ArrayList(std.ArrayList(i32)).init(allocator);
    defer counter_diag_tier.deinit();

    // get the counter diagonal by index sum, i.e.
    // (1, 0) -> 1 + 0 = 1
    // (0, 1) -> 0 + 1 = 1
    for (nums, 0..) |row, row_idx| {
        for (row, 0..) |col_val, col_idx| {
            if (row_idx + col_idx < counter_diag_tier.items.len) {
                var ptr: *std.ArrayList(i32) = &counter_diag_tier.items[row_idx + col_idx];
                try ptr.append(col_val);
            } else {
                var new_row = std.ArrayList(i32).init(allocator);
                defer new_row.deinit();
                try new_row.append(col_val);
                try counter_diag_tier.append(try new_row.clone());
            }
        }
    }

    // reverse the order of each counter diagonal and flatten it
    var res = std.ArrayList(i32).init(allocator);
    defer res.deinit();

    for (counter_diag_tier.items) |row| {
        var col: usize = row.items.len;
        while (col > 0) : (col -= 1) {
            try res.append(row.items[col - 1]);
        }
    }

    // shouldn't be done this way
    // must clone to "circumvent" memory leak issue
    var ret = res.clone();
    return ret;
}
LeetCode

Level up your coding skills and quickly land a job. This is the best place to expand your knowledge and get prepared for your next interview.

#
const std = @import("std");
const diag_travers = @import("diagonal_traverseII.zig");

pub fn main() !void {
    var nums: [3][3]i32 = .{ .{ 1, 2, 3 }, .{ 4, 5, 6 }, .{ 7, 8, 9 } };
    var res = try diag_travers.diagonalTraverseII_bucket(nums);

    for (res.items) |item| {
        std.debug.print("{d}\n", .{item});
    }
}

My problem is, that I still got a memory leak and I'm just working around instead of solving it.
Is there a way to do it better? Or am I missing something?
inner haven
#

Couple of problems:

  • You're initialising the GeneralPurposeAllocator in the function, which keeps metadata on the stack. When you return memory allocated by it you then have no way to free it.
  • You have a list of lists, and you're not deinitialising each sublist
  • Unnecessary clones
  • Using ArrayList instead of ArrayListUnmanaged, which uses less memory per instance as it doesn't store the allocator
  • More memory leaks on allocation failure
#
const std = @import("std");
const Allocator = std.mem.Allocator;

pub fn diagonalTraverseII_bucket(allocator: Allocator, nums: [3][3]i32) !std.ArrayListUnmanaged(i32) {
    var counter_diag_tier: std.ArrayListUnmanaged(std.ArrayListUnmanaged(i32)) = .{};
    defer {
        for (counter_diag_tier.items) |*sublist|
            sublist.deinit(allocator);
        counter_diag_tier.deinit(allocator);
    }

    // get the counter diagonal by index sum, i.e.
    // (1, 0) -> 1 + 0 = 1
    // (0, 1) -> 0 + 1 = 1
    for (nums, 0..) |row, row_idx| {
        for (row, 0..) |col_val, col_idx| {
            if (row_idx + col_idx < counter_diag_tier.items.len) {
                var ptr = &counter_diag_tier.items[row_idx + col_idx];
                try ptr.append(allocator, col_val);
            } else {
                var new_row: std.ArrayListUnmanaged(i32) = .{};
                try new_row.append(allocator, col_val);
                errdefer new_row.deinit(allocator);

                try counter_diag_tier.append(allocator, new_row);
            }
        }
    }

    // reverse the order of each counter diagonal and flatten it
    var res: std.ArrayListUnmanaged(i32) = .{};
    errdefer res.deinit(allocator);

    for (counter_diag_tier.items) |row| {
        var col: usize = row.items.len;
        while (col > 0) : (col -= 1) {
            try res.append(allocator, row.items[col - 1]);
        }
    }

    return res;
}

pub fn main() !void {
    var allocator_gpa = std.heap.GeneralPurposeAllocator(.{}){};
    defer std.debug.print("GPA results: {}\n", .{allocator_gpa.deinit()});
    var logging_alloc = std.heap.loggingAllocator(allocator_gpa.allocator());
    const allocator = logging_alloc.allocator();

    const nums: [3][3]i32 = .{ .{ 1, 2, 3 }, .{ 4, 5, 6 }, .{ 7, 8, 9 } };
    var res = try diagonalTraverseII_bucket(allocator, nums);
    defer res.deinit(allocator);

    for (res.items) |item| {
        std.debug.print("{d}\n", .{item});
    }
}
#

Here's a working version that stays close to your original

#

It also properly frees all memory on allocation failure

inner haven
#

Note that in this case you can actually determine the amount of memory you need to allocate at compile time, so you could further improve that by not using allocators at all - std.BoundedArray would be your friend in that case