From d253beb2f72442f6d05e1b4b0711a37bfe68fdc7 Mon Sep 17 00:00:00 2001 From: Andrew Kaster Date: Wed, 13 Dec 2023 12:10:40 -0700 Subject: [PATCH] LibCore: Don't leak proto-Resources when loading non-existent paths The construct `adopt_ref(*new Obj(TRY(get_resource())))` is another manifestation of a classic anti-pattern. In old C++, you would leak the object's memory if the argument threw an exception, or if a member initializer threw an exception. In our case, we leak if the MappedFile returns an Error. This is pretty concerning, and we should avoid this pattern at all costs, and try to use the "safer" helpers whenever possible. --- Userland/Libraries/LibCore/ResourceImplementation.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Userland/Libraries/LibCore/ResourceImplementation.cpp b/Userland/Libraries/LibCore/ResourceImplementation.cpp index 580843d5a1..e1852b3aee 100644 --- a/Userland/Libraries/LibCore/ResourceImplementation.cpp +++ b/Userland/Libraries/LibCore/ResourceImplementation.cpp @@ -50,9 +50,11 @@ ErrorOr> ResourceImplementation::load_from_uri(StringVie if (uri.starts_with(file_scheme)) { auto path = uri.substring_view(file_scheme.length()); + auto utf8_path = TRY(String::from_utf8(path)); if (is_directory(path)) - return adopt_ref(*new Resource(TRY(String::from_utf8(path)), Resource::Scheme::File, Resource::DirectoryTag {})); - return adopt_ref(*new Resource(TRY(String::from_utf8(path)), Resource::Scheme::File, TRY(MappedFile::map(path)))); + return adopt_ref(*new Resource(utf8_path, Resource::Scheme::File, Resource::DirectoryTag {})); + auto mapped_file = TRY(MappedFile::map(path)); + return adopt_ref(*new Resource(utf8_path, Resource::Scheme::File, move(mapped_file))); } dbgln("ResourceImplementation: Unknown scheme for {}", uri);