From 98b28b338af752e3161b214ea4edbc5d21edef3c Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Wed, 6 Nov 2024 03:05:08 -0800 Subject: [PATCH] Improve duplicate resource error when outputting COFF obj --- src/cvtres.zig | 24 +++++++++++++++---- src/errors.zig | 13 ++-------- src/main.zig | 20 +++++++++++++--- src/res.zig | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++ test/utils.zig | 2 +- 5 files changed, 104 insertions(+), 20 deletions(-) diff --git a/src/cvtres.zig b/src/cvtres.zig index 0b00c9d..71c12e4 100644 --- a/src/cvtres.zig +++ b/src/cvtres.zig @@ -177,12 +177,26 @@ pub const CoffOptions = struct { fold_duplicate_data: bool = false, }; -pub fn writeCoff(allocator: Allocator, writer: anytype, resources: []const Resource, options: CoffOptions) !void { +pub const Diagnostics = union { + none: void, + /// Contains the index of the second resource in a duplicate resource pair. + duplicate_resource: usize, +}; + +pub fn writeCoff(allocator: Allocator, writer: anytype, resources: []const Resource, options: CoffOptions, diagnostics: ?*Diagnostics) !void { var resource_tree = ResourceTree.init(allocator); defer resource_tree.deinit(); for (resources, 0..) |*resource, i| { - try resource_tree.put(resource, i); + resource_tree.put(resource, i) catch |err| { + switch (err) { + error.DuplicateResource => { + if (diagnostics) |d_ptr| d_ptr.* = .{ .duplicate_resource = i }; + }, + else => {}, + } + return err; + }; } const lengths = resource_tree.dataLengths(); @@ -1059,7 +1073,7 @@ test writeCoff { { buf.clearRetainingCapacity(); - try writeCoff(std.testing.allocator, buf.writer(), &.{}, .{}); + try writeCoff(std.testing.allocator, buf.writer(), &.{}, .{}, null); try std.testing.expectEqualSlices( u8, @@ -1082,7 +1096,7 @@ test writeCoff { .characteristics = 0, .data = "foo", }, - }, .{}); + }, .{}, null); try std.testing.expectEqualSlices( u8, @@ -1124,7 +1138,7 @@ fn testResToCoff(res_data: []const u8, expected_coff: []const u8) !void { var buf = try std.ArrayList(u8).initCapacity(std.testing.allocator, expected_coff.len); defer buf.deinit(); - try writeCoff(std.testing.allocator, buf.writer(), resources.list.items, .{}); + try writeCoff(std.testing.allocator, buf.writer(), resources.list.items, .{}, null); try std.testing.expectEqualSlices(u8, expected_coff, buf.items); } diff --git a/src/errors.zig b/src/errors.zig index 7777680..9727872 100644 --- a/src/errors.zig +++ b/src/errors.zig @@ -626,17 +626,8 @@ pub const ErrorDetails = struct { }, .string_already_defined => switch (self.type) { .err, .warning => { - const language_id = self.extra.string_and_language.language.asInt(); - const language_name = language_name: { - if (std.meta.intToEnum(lang.LanguageId, language_id)) |lang_enum_val| { - break :language_name @tagName(lang_enum_val); - } else |_| {} - if (language_id == lang.LOCALE_CUSTOM_UNSPECIFIED) { - break :language_name "LOCALE_CUSTOM_UNSPECIFIED"; - } - break :language_name ""; - }; - return writer.print("string with id {d} (0x{X}) already defined for language {s} (0x{X})", .{ self.extra.string_and_language.id, self.extra.string_and_language.id, language_name, language_id }); + const language = self.extra.string_and_language.language; + return writer.print("string with id {d} (0x{X}) already defined for language {}", .{ self.extra.string_and_language.id, self.extra.string_and_language.id, language }); }, .note => return writer.print("previous definition of string with id {d} (0x{X}) here", .{ self.extra.string_and_language.id, self.extra.string_and_language.id }), .hint => return, diff --git a/src/main.zig b/src/main.zig index 624b7ac..c571169 100644 --- a/src/main.zig +++ b/src/main.zig @@ -15,6 +15,7 @@ const hasDisjointCodePage = @import("disjoint_code_page.zig").hasDisjointCodePag const cvtres = @import("cvtres.zig"); const aro = @import("aro"); const subcommands = @import("subcommands.zig"); +const fmtResourceType = @import("res.zig").NameOrOrdinal.fmtResourceType; pub fn main() !void { var gpa = std.heap.GeneralPurposeAllocator(.{ .stack_trace_frames = 8 }){}; @@ -402,9 +403,22 @@ pub fn main() !void { var coff_output_buffered_stream = std.io.bufferedWriter(coff_output_file.writer()); - cvtres.writeCoff(allocator, coff_output_buffered_stream.writer(), resources.list.items, options.coff_options) catch |err| { - // TODO: Better errors - try renderErrorMessage(stderr.writer(), stderr_config, .err, "unable to write coff output file '{s}': {s}", .{ coff_output_filename, @errorName(err) }); + var cvtres_diagnostics: cvtres.Diagnostics = .{ .none = {} }; + cvtres.writeCoff(allocator, coff_output_buffered_stream.writer(), resources.list.items, options.coff_options, &cvtres_diagnostics) catch |err| { + switch (err) { + error.DuplicateResource => { + const duplicate_resource = resources.list.items[cvtres_diagnostics.duplicate_resource]; + try renderErrorMessage(stderr.writer(), stderr_config, .err, "duplicate resource [id: {}, type: {}, language: {}]", .{ + duplicate_resource.name_value, + fmtResourceType(duplicate_resource.type_value), + duplicate_resource.language, + }); + // TODO: Add note about where the first and second duplicate resources are if additional_inputs.len > 0 + }, + else => { + try renderErrorMessage(stderr.writer(), stderr_config, .err, "unable to write coff output file '{s}': {s}", .{ coff_output_filename, @errorName(err) }); + }, + } // Delete the output file on error coff_output_file.close(); coff_output_file_closed = true; diff --git a/src/res.zig b/src/res.zig index 973b39b..37475b3 100644 --- a/src/res.zig +++ b/src/res.zig @@ -162,6 +162,27 @@ pub const Language = packed struct(u16) { pub fn asInt(self: Language) u16 { return @bitCast(self); } + + pub fn format( + language: Language, + comptime fmt: []const u8, + options: std.fmt.FormatOptions, + out_stream: anytype, + ) !void { + _ = fmt; + _ = options; + const language_id = language.asInt(); + const language_name = language_name: { + if (std.meta.intToEnum(lang.LanguageId, language_id)) |lang_enum_val| { + break :language_name @tagName(lang_enum_val); + } else |_| {} + if (language_id == lang.LOCALE_CUSTOM_UNSPECIFIED) { + break :language_name "LOCALE_CUSTOM_UNSPECIFIED"; + } + break :language_name ""; + }; + try out_stream.print("{s} (0x{X})", .{ language_name, language_id }); + } }; /// https://learn.microsoft.com/en-us/windows/win32/api/winuser/ns-winuser-dlgitemtemplate#remarks @@ -423,6 +444,50 @@ pub const NameOrOrdinal = union(enum) { .name => return null, } } + + pub fn format( + self: NameOrOrdinal, + comptime fmt: []const u8, + options: std.fmt.FormatOptions, + out_stream: anytype, + ) !void { + _ = fmt; + _ = options; + switch (self) { + .name => |name| { + try out_stream.print("{s}", .{std.unicode.fmtUtf16Le(name)}); + }, + .ordinal => |ordinal| { + try out_stream.print("{d}", .{ordinal}); + }, + } + } + + fn formatResourceType( + self: NameOrOrdinal, + comptime fmt: []const u8, + options: std.fmt.FormatOptions, + out_stream: anytype, + ) !void { + _ = fmt; + _ = options; + switch (self) { + .name => |name| { + try out_stream.print("{s}", .{std.unicode.fmtUtf16Le(name)}); + }, + .ordinal => |ordinal| { + if (std.enums.tagName(RT, @enumFromInt(ordinal))) |predefined_type_name| { + try out_stream.print("{s}", .{predefined_type_name}); + } else { + try out_stream.print("{d}", .{ordinal}); + } + }, + } + } + + pub fn fmtResourceType(type_value: NameOrOrdinal) std.fmt.Formatter(formatResourceType) { + return .{ .data = type_value }; + } }; fn expectNameOrOrdinal(expected: NameOrOrdinal, actual: NameOrOrdinal) !void { diff --git a/test/utils.zig b/test/utils.zig index b0cb52d..679975f 100644 --- a/test/utils.zig +++ b/test/utils.zig @@ -300,7 +300,7 @@ pub fn getResinatorCvtResResult(allocator: Allocator, res_source: []const u8, op .target = options.target, .read_only = options.read_only, .define_external_symbol = options.define_external_symbol, - }) catch |err| { + }, null) catch |err| { buf.deinit(); return .{ .coff_err = err,