From da359b8e3664ded902d7287bcb9832e6db45de4f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 19 May 2022 15:49:26 -0700 Subject: [PATCH] properly copy string cli flags --- src/cli_args.zig | 82 +++++++++++++++++++++++++++++++++++------------- src/config.zig | 9 ++++++ src/main.zig | 5 +-- 3 files changed, 73 insertions(+), 23 deletions(-) diff --git a/src/cli_args.zig b/src/cli_args.zig index 0929a5c89..11cdd4a26 100644 --- a/src/cli_args.zig +++ b/src/cli_args.zig @@ -1,6 +1,8 @@ const std = @import("std"); const mem = std.mem; const assert = std.debug.assert; +const Allocator = mem.Allocator; +const ArenaAllocator = std.heap.ArenaAllocator; // TODO: // - Only `--long=value` format is accepted. Do we want to allow @@ -12,10 +14,18 @@ const assert = std.debug.assert; /// the valid CLI flags. See the tests in this file as an example. For field /// types that are structs, the struct can implement the `parseCLI` function /// to do custom parsing. -pub fn parse(comptime T: type, dst: *T, iter: anytype) !void { +pub fn parse(comptime T: type, alloc: Allocator, dst: *T, iter: anytype) !void { const info = @typeInfo(T); assert(info == .Struct); + // Make an arena for all our allocations if we support it. Otherwise, + // use an allocator that always fails. + const arena_alloc = if (@hasField(T, "_arena")) arena: { + dst._arena = ArenaAllocator.init(alloc); + break :arena dst._arena.?.allocator(); + } else std.mem.fail_allocator; + errdefer if (@hasField(T, "_arena")) dst._arena.?.deinit(); + while (iter.next()) |arg| { if (mem.startsWith(u8, arg, "--")) { var key: []const u8 = arg[2..]; @@ -29,13 +39,23 @@ pub fn parse(comptime T: type, dst: *T, iter: anytype) !void { break :value null; }; - try parseIntoField(T, dst, key, value); + try parseIntoField(T, arena_alloc, dst, key, value); } } } /// Parse a single key/value pair into the destination type T. -fn parseIntoField(comptime T: type, dst: *T, key: []const u8, value: ?[]const u8) !void { +/// +/// This may result in allocations. The allocations can only be freed by freeing +/// all the memory associated with alloc. It is expected that alloc points to +/// an arena. +fn parseIntoField( + comptime T: type, + alloc: Allocator, + dst: *T, + key: []const u8, + value: ?[]const u8, +) !void { const info = @typeInfo(T); assert(info == .Struct); @@ -57,7 +77,11 @@ fn parseIntoField(comptime T: type, dst: *T, key: []const u8, value: ?[]const u8 // Otherwise infer based on type break :field switch (Field) { - []const u8 => value orelse return error.ValueRequired, + []const u8 => if (value) |slice| value: { + const buf = try alloc.alloc(u8, slice.len); + mem.copy(u8, buf, slice); + break :value buf; + } else return error.ValueRequired, bool => try parseBool(value orelse "t"), else => unreachable, }; @@ -88,17 +112,21 @@ test "parse: simple" { const testing = std.testing; var data: struct { - a: []const u8, - b: bool, - @"b-f": bool, - } = undefined; + a: []const u8 = "", + b: bool = false, + @"b-f": bool = true, + + _arena: ?ArenaAllocator = null, + } = .{}; + defer if (data._arena) |arena| arena.deinit(); var iter = try std.process.ArgIteratorGeneral(.{}).init( testing.allocator, "--a=42 --b --b-f=false", ); defer iter.deinit(); - try parse(@TypeOf(data), &data, &iter); + try parse(@TypeOf(data), testing.allocator, &data, &iter); + try testing.expect(data._arena != null); try testing.expectEqualStrings("42", data.a); try testing.expect(data.b); try testing.expect(!data.@"b-f"); @@ -106,57 +134,69 @@ test "parse: simple" { test "parseIntoField: string" { const testing = std.testing; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); var data: struct { a: []const u8, } = undefined; - try parseIntoField(@TypeOf(data), &data, "a", "42"); - try testing.expectEqual(@as([]const u8, "42"), data.a); + try parseIntoField(@TypeOf(data), alloc, &data, "a", "42"); + try testing.expectEqualStrings("42", data.a); } test "parseIntoField: bool" { const testing = std.testing; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); var data: struct { a: bool, } = undefined; // True - try parseIntoField(@TypeOf(data), &data, "a", "1"); + try parseIntoField(@TypeOf(data), alloc, &data, "a", "1"); try testing.expectEqual(true, data.a); - try parseIntoField(@TypeOf(data), &data, "a", "t"); + try parseIntoField(@TypeOf(data), alloc, &data, "a", "t"); try testing.expectEqual(true, data.a); - try parseIntoField(@TypeOf(data), &data, "a", "T"); + try parseIntoField(@TypeOf(data), alloc, &data, "a", "T"); try testing.expectEqual(true, data.a); - try parseIntoField(@TypeOf(data), &data, "a", "true"); + try parseIntoField(@TypeOf(data), alloc, &data, "a", "true"); try testing.expectEqual(true, data.a); // False - try parseIntoField(@TypeOf(data), &data, "a", "0"); + try parseIntoField(@TypeOf(data), alloc, &data, "a", "0"); try testing.expectEqual(false, data.a); - try parseIntoField(@TypeOf(data), &data, "a", "f"); + try parseIntoField(@TypeOf(data), alloc, &data, "a", "f"); try testing.expectEqual(false, data.a); - try parseIntoField(@TypeOf(data), &data, "a", "F"); + try parseIntoField(@TypeOf(data), alloc, &data, "a", "F"); try testing.expectEqual(false, data.a); - try parseIntoField(@TypeOf(data), &data, "a", "false"); + try parseIntoField(@TypeOf(data), alloc, &data, "a", "false"); try testing.expectEqual(false, data.a); } test "parseIntoField: optional field" { const testing = std.testing; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); var data: struct { a: ?bool = null, } = .{}; // True - try parseIntoField(@TypeOf(data), &data, "a", "1"); + try parseIntoField(@TypeOf(data), alloc, &data, "a", "1"); try testing.expectEqual(true, data.a.?); } test "parseIntoField: struct with parse func" { const testing = std.testing; + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); var data: struct { a: struct { @@ -171,6 +211,6 @@ test "parseIntoField: struct with parse func" { }, } = undefined; - try parseIntoField(@TypeOf(data), &data, "a", "42"); + try parseIntoField(@TypeOf(data), alloc, &data, "a", "42"); try testing.expectEqual(@as([]const u8, "HELLO!"), data.a.v); } diff --git a/src/config.zig b/src/config.zig index 2df4b75fa..47297b803 100644 --- a/src/config.zig +++ b/src/config.zig @@ -1,4 +1,5 @@ const std = @import("std"); +const ArenaAllocator = std.heap.ArenaAllocator; pub const Config = struct { /// Background color for the window. @@ -10,6 +11,14 @@ pub const Config = struct { /// The command to run, usually a shell. If this is not an absolute path, /// it'll be looked up in the PATH. command: ?[]const u8 = null, + + /// This is set by the CLI parser for deinit. + _arena: ?ArenaAllocator = null, + + pub fn deinit(self: *Config) void { + if (self._arena) |arena| arena.deinit(); + self.* = undefined; + } }; /// Color represents a color using RGB. diff --git a/src/main.zig b/src/main.zig index 4ef31ce6a..5d715b0b4 100644 --- a/src/main.zig +++ b/src/main.zig @@ -16,13 +16,14 @@ pub fn main() !void { const alloc = if (!tracy.enabled) gpa else tracy.allocator(gpa, null).allocator(); // Parse the config from the CLI args - const config = config: { + var config = config: { var result: Config = .{}; var iter = try std.process.argsWithAllocator(alloc); defer iter.deinit(); - try cli_args.parse(Config, &result, &iter); + try cli_args.parse(Config, alloc, &result, &iter); break :config result; }; + defer config.deinit(); // Initialize glfw try glfw.init(.{});