Skip to content

Commit

Permalink
AK+Everywhere: Add and use static APIs for LexicalPath
Browse files Browse the repository at this point in the history
The LexicalPath instance methods dirname(), basename(), title() and
extension() will be changed to return StringView const& in a further
commit. Due to this, users creating temporary LexicalPath objects just
to call one of those getters will recieve a StringView const& pointing
to a possible freed buffer.

To avoid this, static methods for those APIs have been added, which will
return a String by value to avoid those problems. All cases where
temporary LexicalPath objects have been used as described above haven
been changed to use the static APIs.
  • Loading branch information
MaxWipfli authored and awesomekling committed Jun 30, 2021
1 parent 9b8f352 commit fc6d051
Show file tree
Hide file tree
Showing 43 changed files with 80 additions and 56 deletions.
24 changes: 24 additions & 0 deletions AK/LexicalPath.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,30 @@ class LexicalPath {
return LexicalPath { builder.to_string() };
}

static String dirname(String path)
{
auto lexical_path = LexicalPath(move(path));
return lexical_path.dirname();
}

static String basename(String path)
{
auto lexical_path = LexicalPath(move(path));
return lexical_path.basename();
}

static String title(String path)
{
auto lexical_path = LexicalPath(move(path));
return lexical_path.title();
}

static String extension(String path)
{
auto lexical_path = LexicalPath(move(path));
return lexical_path.extension();
}

private:
void canonicalize();

Expand Down
10 changes: 5 additions & 5 deletions Kernel/FileSystem/VirtualFileSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ KResult VFS::rename(StringView old_path, StringView new_path, Custody& base)
if (old_parent_custody->is_readonly() || new_parent_custody->is_readonly())
return EROFS;

auto new_basename = LexicalPath(new_path).basename();
auto new_basename = LexicalPath::basename(new_path);

if (!new_custody_or_error.is_error()) {
auto& new_custody = *new_custody_or_error.value();
Expand All @@ -554,7 +554,7 @@ KResult VFS::rename(StringView old_path, StringView new_path, Custody& base)
if (auto result = new_parent_inode.add_child(old_inode, new_basename, old_inode.mode()); result.is_error())
return result;

if (auto result = old_parent_inode.remove_child(LexicalPath(old_path).basename()); result.is_error())
if (auto result = old_parent_inode.remove_child(LexicalPath::basename(old_path)); result.is_error())
return result;

return KSuccess;
Expand Down Expand Up @@ -656,7 +656,7 @@ KResult VFS::link(StringView old_path, StringView new_path, Custody& base)
if (!hard_link_allowed(old_inode))
return EPERM;

return parent_inode.add_child(old_inode, LexicalPath(new_path).basename(), old_inode.mode());
return parent_inode.add_child(old_inode, LexicalPath::basename(new_path), old_inode.mode());
}

KResult VFS::unlink(StringView path, Custody& base)
Expand Down Expand Up @@ -689,7 +689,7 @@ KResult VFS::unlink(StringView path, Custody& base)
if (parent_custody->is_readonly())
return EROFS;

if (auto result = parent_inode.remove_child(LexicalPath(path).basename()); result.is_error())
if (auto result = parent_inode.remove_child(LexicalPath::basename(path)); result.is_error())
return result;

return KSuccess;
Expand Down Expand Up @@ -771,7 +771,7 @@ KResult VFS::rmdir(StringView path, Custody& base)
if (auto result = inode.remove_child(".."); result.is_error())
return result;

return parent_inode.remove_child(LexicalPath(path).basename());
return parent_inode.remove_child(LexicalPath::basename(path));
}

VFS::Mount::Mount(FS& guest_fs, Custody* host_custody, int flags)
Expand Down
2 changes: 1 addition & 1 deletion Kernel/Syscalls/unveil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ KResultOr<FlatPtr> Process::sys$unveil(Userspace<const Syscall::SC_unveil_params
if (!custody_or_error.is_error()) {
new_unveiled_path = custody_or_error.value()->absolute_path();
} else if (custody_or_error.error() == -ENOENT && parent_custody && (new_permissions & UnveilAccess::CreateOrRemove)) {
String basename = LexicalPath(path.view()).basename();
String basename = LexicalPath::basename(path.view());
new_unveiled_path = String::formatted("{}/{}", parent_custody->absolute_path(), basename);
} else {
// FIXME Should this be EINVAL?
Expand Down
2 changes: 1 addition & 1 deletion Userland/Applications/CrashReporter/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ int main(int argc, char** argv)
auto& icon_image_widget = *widget.find_descendant_of_type_named<GUI::ImageWidget>("icon");
icon_image_widget.set_bitmap(GUI::FileIconProvider::icon_for_executable(executable_path).bitmap_for_size(32));

auto app_name = LexicalPath(executable_path).basename();
auto app_name = LexicalPath::basename(executable_path);
auto af = Desktop::AppFile::get_for_app(app_name);
if (af->is_valid())
app_name = af->name();
Expand Down
4 changes: 2 additions & 2 deletions Userland/Applications/FileManager/DirectoryView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ static void run_file_operation([[maybe_unused]] FileOperation operation, const S
perror("dup2");
_exit(1);
}
if (execlp("/bin/FileOperation", "/bin/FileOperation", "Copy", source.characters(), LexicalPath(destination).dirname().characters(), nullptr) < 0) {
if (execlp("/bin/FileOperation", "/bin/FileOperation", "Copy", source.characters(), LexicalPath::dirname(destination).characters(), nullptr) < 0) {
perror("execlp");
_exit(1);
}
Expand Down Expand Up @@ -609,7 +609,7 @@ void DirectoryView::handle_drop(const GUI::ModelIndex& index, const GUI::DropEve
for (auto& url_to_copy : urls) {
if (!url_to_copy.is_valid() || url_to_copy.path() == target_node.full_path())
continue;
auto new_path = String::formatted("{}/{}", target_node.full_path(), LexicalPath(url_to_copy.path()).basename());
auto new_path = String::formatted("{}/{}", target_node.full_path(), LexicalPath::basename(url_to_copy.path()));
if (url_to_copy.path() == new_path)
continue;

Expand Down
2 changes: 1 addition & 1 deletion Userland/Applications/FileManager/FileUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ void delete_paths(const Vector<String>& paths, bool should_confirm, GUI::Window*
{
String message;
if (paths.size() == 1) {
message = String::formatted("Really delete {}?", LexicalPath(paths[0]).basename());
message = String::formatted("Really delete {}?", LexicalPath::basename(paths[0]));
} else {
message = String::formatted("Really delete {} files?", paths.size());
}
Expand Down
6 changes: 3 additions & 3 deletions Userland/Applications/FileManager/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ void do_paste(const String& target_directory, GUI::Window* window)
void do_create_link(const Vector<String>& selected_file_paths, GUI::Window* window)
{
auto path = selected_file_paths.first();
auto destination = String::formatted("{}/{}", Core::StandardPaths::desktop_directory(), LexicalPath { path }.basename());
auto destination = String::formatted("{}/{}", Core::StandardPaths::desktop_directory(), LexicalPath::basename(path));
if (auto result = Core::File::link_file(destination, path); result.is_error()) {
GUI::MessageBox::show(window, String::formatted("Could not create desktop shortcut:\n{}", result.error()), "File Manager",
GUI::MessageBox::Type::Error);
Expand Down Expand Up @@ -736,7 +736,7 @@ int run_in_windowed_mode(RefPtr<Core::ConfigFile> config, String initial_locatio
selected = directory_view.selected_file_paths();
} else {
path = directories_model->full_path(tree_view.selection().first());
container_dir_path = LexicalPath(path).basename();
container_dir_path = LexicalPath::basename(path);
selected = tree_view_selected_file_paths();
}

Expand Down Expand Up @@ -1115,7 +1115,7 @@ int run_in_windowed_mode(RefPtr<Core::ConfigFile> config, String initial_locatio
for (auto& url_to_copy : urls) {
if (!url_to_copy.is_valid() || url_to_copy.path() == directory)
continue;
auto new_path = String::formatted("{}/{}", directory, LexicalPath(url_to_copy.path()).basename());
auto new_path = String::formatted("{}/{}", directory, LexicalPath::basename(url_to_copy.path()));
if (url_to_copy.path() == new_path)
continue;

Expand Down
2 changes: 1 addition & 1 deletion Userland/Applications/PixelPaint/Image.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ void Image::set_title(String title)
void Image::set_path(String path)
{
m_path = move(path);
set_title(LexicalPath(m_path).basename());
set_title(LexicalPath::basename(m_path));
}

}
2 changes: 1 addition & 1 deletion Userland/Applications/SoundPlayer/PlaylistWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ GUI::Variant PlaylistModel::data(const GUI::ModelIndex& index, GUI::ModelRole ro
if (role == GUI::ModelRole::Display) {
switch (index.column()) {
case 0:
return m_playlist_items[index.row()].extended_info->track_display_title.value_or(LexicalPath(m_playlist_items[index.row()].path).title());
return m_playlist_items[index.row()].extended_info->track_display_title.value_or(LexicalPath::title(m_playlist_items[index.row()].path));
case 1:
return format_duration(m_playlist_items[index.row()].extended_info->track_length_in_seconds.value_or(0));
case 2:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ void SoundPlayerWidgetAdvancedView::try_fill_missing_info(Vector<M3UEntry>& entr
}

if (!entry.extended_info->track_display_title.has_value())
entry.extended_info->track_display_title = LexicalPath(entry.path).title();
entry.extended_info->track_display_title = LexicalPath::title(entry.path);
if (!entry.extended_info->track_length_in_seconds.has_value()) {
if (auto reader = Audio::Loader::create(entry.path); !reader->has_error())
entry.extended_info->track_length_in_seconds = reader->total_samples() / reader->sample_rate();
Expand Down
2 changes: 1 addition & 1 deletion Userland/Applications/Spreadsheet/ExportDialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ Result<void, String> ExportDialog::make_and_run_for(StringView mime, Core::File&
} else {
auto page = GUI::WizardPage::construct(
"Export File Format",
String::formatted("Select the format you wish to export to '{}' as", LexicalPath { file.filename() }.basename()));
String::formatted("Select the format you wish to export to '{}' as", LexicalPath::basename(file.filename())));

page->on_next_page = [] { return nullptr; };

Expand Down
4 changes: 2 additions & 2 deletions Userland/Applications/Spreadsheet/HelpWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ HelpWindow::HelpWindow(GUI::Window* parent)
m_webview->on_link_click = [this](auto& url, auto&, auto&&) {
VERIFY(url.protocol() == "spreadsheet");
if (url.host() == "example") {
auto entry = LexicalPath(url.path()).basename();
auto entry = LexicalPath::basename(url.path());
auto doc_option = m_docs.get(entry);
if (!doc_option.is_object()) {
GUI::MessageBox::show_error(this, String::formatted("No documentation entry found for '{}'", url.path()));
Expand Down Expand Up @@ -120,7 +120,7 @@ HelpWindow::HelpWindow(GUI::Window* parent)
widget.add_sheet(sheet.release_nonnull());
window->show();
} else if (url.host() == "doc") {
auto entry = LexicalPath(url.path()).basename();
auto entry = LexicalPath::basename(url.path());
m_webview->load(URL::create_with_data("text/html", render(entry)));
} else {
dbgln("Invalid spreadsheet action domain '{}'", url.host());
Expand Down
2 changes: 1 addition & 1 deletion Userland/Applications/Spreadsheet/ImportDialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ Result<NonnullRefPtrVector<Sheet>, String> ImportDialog::make_and_run_for(String
} else {
auto page = GUI::WizardPage::construct(
"Import File Format",
String::formatted("Select the format you wish to import '{}' as", LexicalPath { file.filename() }.basename()));
String::formatted("Select the format you wish to import '{}' as", LexicalPath::basename(file.filename())));

page->on_next_page = [] { return nullptr; };

Expand Down
2 changes: 1 addition & 1 deletion Userland/DevTools/HackStudio/CodeDocument.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ CodeDocument::CodeDocument(const String& file_path, Client* client)
: TextDocument(client)
, m_file_path(file_path)
{
m_language = language_from_file_extension(LexicalPath { file_path }.extension());
m_language = language_from_file_extension(LexicalPath::extension(file_path));
}

CodeDocument::CodeDocument(Client* client)
Expand Down
2 changes: 1 addition & 1 deletion Userland/DevTools/HackStudio/Editor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ static HashMap<String, String>& man_paths()
Core::DirIterator it("/usr/share/man/man2", Core::DirIterator::Flags::SkipDots);
while (it.has_next()) {
auto path = it.next_full_path();
auto title = LexicalPath(path).title();
auto title = LexicalPath::title(path);
paths.set(title, path);
}
}
Expand Down
2 changes: 1 addition & 1 deletion Userland/DevTools/HackStudio/HackStudioWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ String HackStudioWidget::get_project_executable_path() const
// FIXME: Dumb heuristic ahead!
// e.g /my/project => /my/project/project
// TODO: Perhaps a Makefile rule for getting the value of $(PROGRAM) would be better?
return String::formatted("{}/{}", m_project->root_path(), LexicalPath(m_project->root_path()).basename());
return String::formatted("{}/{}", m_project->root_path(), LexicalPath::basename(m_project->root_path()));
}

void HackStudioWidget::build(TerminalWrapper& wrapper)
Expand Down
2 changes: 1 addition & 1 deletion Userland/DevTools/HackStudio/Project.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class Project {

GUI::FileSystemModel& model() { return *m_model; }
const GUI::FileSystemModel& model() const { return *m_model; }
String name() const { return LexicalPath(m_root_path).basename(); }
String name() const { return LexicalPath::basename(m_root_path); }
String root_path() const { return m_root_path; }

NonnullRefPtr<ProjectFile> get_file(const String& path) const;
Expand Down
2 changes: 1 addition & 1 deletion Userland/DevTools/HackStudio/ProjectTemplate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ RefPtr<ProjectTemplate> ProjectTemplate::load_from_manifest(const String& manife
|| !config->has_key("HackStudioTemplate", "IconName32x"))
return {};

auto id = LexicalPath(manifest_path).title();
auto id = LexicalPath::title(manifest_path);
auto name = config->read_entry("HackStudioTemplate", "Name");
auto description = config->read_entry("HackStudioTemplate", "Description");
int priority = config->read_num_entry("HackStudioTemplate", "Priority", 0);
Expand Down
6 changes: 3 additions & 3 deletions Userland/DevTools/Profiler/Profile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ Result<NonnullOwnPtr<Profile>, String> Profile::load_from_perfcore_file(const St
auto sampled_process = adopt_own(*new Process {
.pid = event.pid,
.executable = event.executable,
.basename = LexicalPath(event.executable).basename(),
.basename = LexicalPath::basename(event.executable),
.start_valid = event.serial,
.end_valid = {},
});
Expand All @@ -274,7 +274,7 @@ Result<NonnullOwnPtr<Profile>, String> Profile::load_from_perfcore_file(const St
auto sampled_process = adopt_own(*new Process {
.pid = event.pid,
.executable = event.executable,
.basename = LexicalPath(event.executable).basename(),
.basename = LexicalPath::basename(event.executable),
.start_valid = event.serial,
.end_valid = {},
});
Expand Down Expand Up @@ -498,7 +498,7 @@ ProfileNode::ProfileNode(Process const& process, const String& object_name, Stri
} else {
object = object_name;
}
m_object_name = LexicalPath(object).basename();
m_object_name = LexicalPath::basename(object);
}

}
2 changes: 1 addition & 1 deletion Userland/DevTools/Profiler/TimelineHeader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ TimelineHeader::TimelineHeader(Profile& profile, Process const& process)
update_selection();

m_icon = GUI::FileIconProvider::icon_for_executable(m_process.executable).bitmap_for_size(32);
m_text = String::formatted("{} ({})", LexicalPath(m_process.executable).basename(), m_process.pid);
m_text = String::formatted("{} ({})", LexicalPath::basename(m_process.executable), m_process.pid);
}

TimelineHeader::~TimelineHeader()
Expand Down
4 changes: 2 additions & 2 deletions Userland/DevTools/UserspaceEmulator/Emulator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ String Emulator::create_backtrace_line(FlatPtr address)

auto source_position = it->value.debug_info->get_source_position(address - region->base());
if (source_position.has_value())
return String::formatted("=={{{}}}== {:p} [{}]: {} (\e[34;1m{}\e[0m:{})", getpid(), (void*)address, lib_name, symbol, LexicalPath(source_position.value().file_path).basename(), source_position.value().line_number);
return String::formatted("=={{{}}}== {:p} [{}]: {} (\e[34;1m{}\e[0m:{})", getpid(), (void*)address, lib_name, symbol, LexicalPath::basename(source_position.value().file_path), source_position.value().line_number);

return line_without_source_info;
}
Expand Down Expand Up @@ -464,7 +464,7 @@ String Emulator::create_instruction_line(FlatPtr address, X86::Instruction insn)
if (!source_position.has_value())
return minimal;

return String::formatted("{:p}: {} \e[34;1m{}\e[0m:{}", (void*)address, insn.to_string(address), LexicalPath(source_position.value().file_path).basename(), source_position.value().line_number);
return String::formatted("{:p}: {} \e[34;1m{}\e[0m:{}", (void*)address, insn.to_string(address), LexicalPath::basename(source_position.value().file_path), source_position.value().line_number);
}

static void emulator_signal_handler(int signum)
Expand Down
2 changes: 1 addition & 1 deletion Userland/DevTools/UserspaceEmulator/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ int main(int argc, char** argv, char** env)

StringBuilder builder;
builder.append("(UE) ");
builder.append(LexicalPath(arguments[0]).basename());
builder.append(LexicalPath::basename(arguments[0]));
if (set_process_name(builder.string_view().characters_without_null_termination(), builder.string_view().length()) < 0) {
perror("set_process_name");
return 1;
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibCore/File.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ Result<void, File::CopyError> File::copy_file(const String& dst_path, const stru
if (errno != EISDIR)
return CopyError { OSError(errno), false };

auto dst_dir_path = String::formatted("{}/{}", dst_path, LexicalPath(source.filename()).basename());
auto dst_dir_path = String::formatted("{}/{}", dst_path, LexicalPath::basename(source.filename()));
dst_fd = creat(dst_dir_path.characters(), 0666);
if (dst_fd < 0)
return CopyError { OSError(errno), false };
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibCoreDump/Backtrace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ String Backtrace::Entry::to_string(bool color) const
for (size_t i = 0; i < source_positions.size(); ++i) {
auto& position = source_positions[i];
auto fmt = color ? "\033[34;1m{}\033[0m:{}" : "{}:{}";
builder.appendff(fmt, LexicalPath(position.file_path).basename(), position.line_number);
builder.appendff(fmt, LexicalPath::basename(position.file_path), position.line_number);
if (i != source_positions.size() - 1) {
builder.append(" => ");
}
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibDebug/DebugSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ void DebugSession::update_loaded_libs()

String lib_name = object_path.value();
if (lib_name.ends_with(".so"))
lib_name = LexicalPath(object_path.value()).basename();
lib_name = LexicalPath::basename(object_path.value());

// FIXME: DebugInfo currently cannot parse the debug information of libgcc_s.so
if (lib_name == "libgcc_s.so")
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibELF/DynamicLinker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ Optional<DynamicObject::SymbolLookupResult> DynamicLinker::lookup_global_symbol(

static String get_library_name(String path)
{
return LexicalPath(move(path)).basename();
return LexicalPath::basename(move(path));
}

static Result<NonnullRefPtr<DynamicLoader>, DlErrorMessage> map_library(const String& filename, int fd)
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibGUI/FileIconProvider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ Icon FileIconProvider::icon_for_path(const String& path, mode_t mode)
if (raw_symlink_target.starts_with('/')) {
target_path = raw_symlink_target;
} else {
target_path = Core::File::real_path_for(String::formatted("{}/{}", LexicalPath(path).dirname(), raw_symlink_target));
target_path = Core::File::real_path_for(String::formatted("{}/{}", LexicalPath::dirname(path), raw_symlink_target));
}
auto target_icon = icon_for_path(target_path);

Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibGUI/FileSystemModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,7 @@ void FileSystemModel::set_data(const ModelIndex& index, const Variant& data)
{
VERIFY(is_editable(index));
Node& node = const_cast<Node&>(this->node(index));
auto dirname = LexicalPath(node.full_path()).dirname();
auto dirname = LexicalPath::dirname(node.full_path());
auto new_full_path = String::formatted("{}/{}", dirname, data.to_string());
int rc = rename(node.full_path().characters(), new_full_path.characters());
if (rc < 0) {
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibTest/JavaScriptTestRunnerMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ int main(int argc, char** argv)
{
g_test_argc = argc;
g_test_argv = argv;
auto program_name = LexicalPath { argv[0] }.basename();
auto program_name = LexicalPath::basename(argv[0]);
g_program_name = program_name;

struct sigaction act;
Expand Down
Loading

0 comments on commit fc6d051

Please sign in to comment.