From aa95e78a80d522e375b292d50bd280a5f2de5c81 Mon Sep 17 00:00:00 2001 From: CJ van den Berg Date: Mon, 6 Jan 2025 12:09:11 +0100 Subject: [PATCH] refactor(nested config files): simplify and avoid duplicate code Also, fix a small use after free bug. --- src/main.zig | 61 +++++++++++++++++++++++++++++++--------------- src/tui/tui.zig | 64 +++---------------------------------------------- 2 files changed, 45 insertions(+), 80 deletions(-) diff --git a/src/main.zig b/src/main.zig index 72b5164..390083b 100644 --- a/src/main.zig +++ b/src/main.zig @@ -391,44 +391,67 @@ pub fn exit(status: u8) noreturn { const config = @import("config"); -pub fn read_config(allocator: std.mem.Allocator, buf: *?[]const u8) config { - const file_name = get_app_config_file_name(application_name) catch return .{}; - return read_config_at(allocator, buf, file_name); -} -pub fn read_config_at(allocator: std.mem.Allocator, buf: *?[]const u8, file_name: []const u8) config { - return read_json_config_file(allocator, file_name, buf) catch |e| { - log.logger("config").print_err("read_config", "error reading config file: {any}", .{e}); - return .{}; - }; +pub fn free_config(allocator: std.mem.Allocator, bufs: [][]const u8) void { + for (bufs) |buf| allocator.free(buf); } -fn read_json_config_file(allocator: std.mem.Allocator, file_name: []const u8, buf: *?[]const u8) !config { +pub fn read_config(allocator: std.mem.Allocator) struct { config, [][]const u8 } { + var bufs: [][]const u8 = &[_][]const u8{}; + const file_name = get_app_config_file_name(application_name) catch return .{ .{}, bufs }; + var conf: config = .{}; + read_config_file(allocator, &conf, &bufs, file_name); + read_nested_config_files(allocator, &conf, &bufs); + return .{ conf, bufs }; +} + +fn read_config_file(allocator: std.mem.Allocator, conf: *config, bufs: *[][]const u8, file_name: []const u8) void { + read_json_config_file(allocator, conf, bufs, file_name) catch |e| + log.logger("config").print_err("read_config", "error reading config file: {any}", .{e}); + return; +} + +fn read_json_config_file(allocator: std.mem.Allocator, conf: *config, bufs_: *[][]const u8, file_name: []const u8) !void { const cbor = @import("cbor"); var file = std.fs.openFileAbsolute(file_name, .{ .mode = .read_only }) catch |e| switch (e) { - error.FileNotFound => return .{}, + error.FileNotFound => return, else => return e, }; defer file.close(); const json = try file.readToEndAlloc(allocator, 64 * 1024); defer allocator.free(json); const cbor_buf: []u8 = try allocator.alloc(u8, json.len); - buf.* = cbor_buf; + var bufs = std.ArrayListUnmanaged([]const u8).fromOwnedSlice(bufs_.*); + bufs.append(allocator, cbor_buf) catch @panic("OOM:read_json_config_file"); + bufs_.* = bufs.toOwnedSlice(allocator) catch @panic("OOM:read_json_config_file"); const cb = try cbor.fromJson(json, cbor_buf); var iter = cb; var len = try cbor.decodeMapHeader(&iter); - var data: config = .{}; while (len > 0) : (len -= 1) { var field_name: []const u8 = undefined; if (!(try cbor.matchString(&iter, &field_name))) return error.InvalidConfig; - inline for (@typeInfo(config).Struct.fields) |field_info| { - if (std.mem.eql(u8, field_name, field_info.name)) { + inline for (@typeInfo(config).Struct.fields) |field_info| + if (comptime std.mem.eql(u8, "config_files", field_info.name)) { + if (std.mem.eql(u8, field_name, field_info.name)) { + var value: field_info.type = undefined; + if (!(try cbor.matchValue(&iter, cbor.extract(&value)))) return error.InvalidConfig; + if (conf.config_files.len > 0) { + std.log.err("{s}: ignoring nested 'config_files' value '{s}'", .{ file_name, value }); + } else { + @field(conf, field_info.name) = value; + } + } + } else if (std.mem.eql(u8, field_name, field_info.name)) { var value: field_info.type = undefined; if (!(try cbor.matchValue(&iter, cbor.extract(&value)))) return error.InvalidConfig; - @field(data, field_info.name) = value; - } - } + @field(conf, field_info.name) = value; + }; } - return data; +} + +fn read_nested_config_files(allocator: std.mem.Allocator, conf: *config, bufs: *[][]const u8) void { + if (conf.config_files.len == 0) return; + var it = std.mem.splitScalar(u8, conf.config_files, std.fs.path.delimiter); + while (it.next()) |path| read_config_file(allocator, conf, bufs, path); } pub fn write_config(conf: config, allocator: std.mem.Allocator) !void { diff --git a/src/tui/tui.zig b/src/tui/tui.zig index d860797..02d6ca8 100644 --- a/src/tui/tui.zig +++ b/src/tui/tui.zig @@ -83,20 +83,8 @@ fn start(args: StartArgs) tp.result { fn init(allocator: Allocator) !*Self { var self = try allocator.create(Self); - var conf_bufs: std.ArrayListUnmanaged([]const u8) = .{}; - defer { - for (conf_bufs.items) |buf| { - allocator.free(buf); - } - conf_bufs.deinit(allocator); - } - var conf = blk: { - var maybe_buf: ?[]const u8 = null; - const conf = root.read_config(allocator, &maybe_buf); - if (maybe_buf) |buf| try conf_bufs.append(allocator, buf); - break :blk conf; - }; - try loadConfigFiles(allocator, &conf_bufs, &conf, conf.config_files); + var conf, const conf_bufs = root.read_config(allocator); + defer root.free_config(allocator, conf_bufs); const theme = get_theme_by_name(conf.theme) orelse get_theme_by_name("dark_modern") orelse return tp.exit("unknown theme"); conf.theme = theme.name; @@ -104,6 +92,7 @@ fn init(allocator: Allocator) !*Self { conf.input_mode = try allocator.dupe(u8, conf.input_mode); conf.top_bar = try allocator.dupe(u8, conf.top_bar); conf.bottom_bar = try allocator.dupe(u8, conf.bottom_bar); + conf.config_files = try allocator.dupe(u8, conf.config_files); if (build_options.gui) conf.enable_terminal_cursor = false; const frame_rate: usize = @intCast(tp.env.get().num("frame-rate")); @@ -223,53 +212,6 @@ fn deinit(self: *Self) void { self.allocator.destroy(self); } -fn loadConfigFiles( - allocator: std.mem.Allocator, - conf_bufs: *std.ArrayListUnmanaged([]const u8), - final_conf: *config, - config_files: []const u8, -) error{OutOfMemory}!void { - if (config_files.len == 0) return; - - var it = std.mem.splitScalar(u8, config_files, std.fs.path.delimiter); - while (it.next()) |path| { - var maybe_buf: ?[]const u8 = null; - const conf = root.read_config_at(allocator, &maybe_buf, path); - if (maybe_buf) |buf| try conf_bufs.append(allocator, buf); - - if (conf.config_files.len > 0) { - // just disallow nested config for now as we'd have to establish some - // sort of priority - std.log.err("{s}: ignoring nested 'config_files'", .{path}); - } - - inline for (std.meta.fields(config)) |field| { - if (comptime std.mem.eql(u8, field.name, "config_files")) continue; - - const new_value_ref = &@field(conf, field.name); - const default_value_ref: *const field.type = @alignCast(@ptrCast(field.default_value)); - const is_default = switch (field.type) { - bool, usize => new_value_ref.* == default_value_ref.*, - []const u8 => std.mem.eql(u8, new_value_ref.*, default_value_ref.*), - else => @compileError("unsupported type: " ++ @typeName(field.type)), - }; - const value_spec: []const u8 = switch (field.type) { - bool, usize => "", - []const u8 => "s", - else => @compileError("unsupported type: " ++ @typeName(field.type)), - }; - if (!is_default) { - std.log.info("{s} override {s} with {" ++ value_spec ++ "}", .{ - path, - field.name, - new_value_ref.*, - }); - @field(final_conf, field.name) = new_value_ref.*; - } - } - } -} - fn listen_sigwinch(self: *Self) tp.result { if (self.sigwinch_signal) |old| old.deinit(); self.sigwinch_signal = tp.signal.init(std.posix.SIG.WINCH, tp.message.fmt(.{"sigwinch"})) catch |e| return tp.exit_error(e, @errorReturnTrace());