From de18485a2f38119de0ff1f1f562f06f81f75f793 Mon Sep 17 00:00:00 2001
From: Caoimhe <caoimhebyrne06@gmail.com>
Date: Sat, 18 Mar 2023 00:05:35 +0000
Subject: [PATCH] LibCore: Improve the `TempFile` wrapper

- We were using primitive versions of mkstemp and mkdtemp, they have
  been converted to use LibCore::System.
- If an error occurred whilst creating a temporary directory or file, it
  was thrown and the program would crash. Now, we use ErrorOr<T> so that
  the caller can handle the error accordingly
- The `Type` enumeration has been made private, and `create_temp` has
  been "split" (although rewritten) into create_temp_directory and
  create_temp_file. The old pattern of TempFile::create_temp(Type::File)
  felt a bit awkward, and TempFile::create_temp_file() feels a bit nicer
  to use! :^)

Once the Core::Filesystem PR is merged (#17789), it would be better for
this helper to be merged in with that. But until then, this is a nice
improvement.
---
 Userland/Libraries/LibCore/TempFile.cpp | 51 +++++++++----------------
 Userland/Libraries/LibCore/TempFile.h   | 34 +++++++++--------
 2 files changed, 37 insertions(+), 48 deletions(-)

diff --git a/Userland/Libraries/LibCore/TempFile.cpp b/Userland/Libraries/LibCore/TempFile.cpp
index 527ad911e41..8558847e43f 100644
--- a/Userland/Libraries/LibCore/TempFile.cpp
+++ b/Userland/Libraries/LibCore/TempFile.cpp
@@ -1,57 +1,42 @@
 /*
- * Copyright (c) 2020-2021, the SerenityOS developers.
+ * Copyright (c) 2020-2023, the SerenityOS developers.
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
 #include "TempFile.h"
-#include <AK/Random.h>
 #include <LibCore/DeprecatedFile.h>
-#include <fcntl.h>
-#include <stdlib.h>
-#include <sys/stat.h>
+#include <LibCore/System.h>
 
 namespace Core {
 
-NonnullOwnPtr<TempFile> TempFile::create(Type type)
+ErrorOr<NonnullOwnPtr<TempFile>> TempFile::create_temp_directory()
 {
-    return adopt_own(*new TempFile(type));
+    char pattern[] = "/tmp/tmp.XXXXXX";
+
+    auto path = TRY(Core::System::mkdtemp(pattern));
+    return adopt_nonnull_own_or_enomem(new (nothrow) TempFile(Type::Directory, path));
 }
 
-DeprecatedString TempFile::create_temp(Type type)
+ErrorOr<NonnullOwnPtr<TempFile>> TempFile::create_temp_file()
 {
-    char name_template[] = "/tmp/tmp.XXXXXX";
-    switch (type) {
-    case Type::File: {
-        auto fd = mkstemp(name_template);
-        VERIFY(fd >= 0);
-        close(fd);
-        break;
-    }
-    case Type::Directory: {
-        auto fd = mkdtemp(name_template);
-        VERIFY(fd != nullptr);
-        break;
-    }
-    }
-    return DeprecatedString { name_template };
-}
+    char file_path[] = "/tmp/tmp.XXXXXX";
+    TRY(Core::System::mkstemp(file_path));
 
-TempFile::TempFile(Type type)
-    : m_type(type)
-    , m_path(create_temp(type))
-{
+    auto string = TRY(String::from_utf8({ file_path, sizeof file_path }));
+    return adopt_nonnull_own_or_enomem(new (nothrow) TempFile(Type::File, string));
 }
 
 TempFile::~TempFile()
 {
-    DeprecatedFile::RecursionMode recursion_allowed { DeprecatedFile::RecursionMode::Disallowed };
+    // Temporary files aren't removed by anyone else, so we must do it ourselves.
+    auto recursion_mode = DeprecatedFile::RecursionMode::Disallowed;
     if (m_type == Type::Directory)
-        recursion_allowed = DeprecatedFile::RecursionMode::Allowed;
+        recursion_mode = DeprecatedFile::RecursionMode::Allowed;
 
-    auto rc = DeprecatedFile::remove(m_path, recursion_allowed);
-    if (rc.is_error()) {
-        warnln("File::remove failed: {}", rc.error().string_literal());
+    auto result = DeprecatedFile::remove(m_path, recursion_mode);
+    if (result.is_error()) {
+        warnln("Removal of temporary file failed: {}", result.error().string_literal());
     }
 }
 
diff --git a/Userland/Libraries/LibCore/TempFile.h b/Userland/Libraries/LibCore/TempFile.h
index 014b36866b5..ac1743a1659 100644
--- a/Userland/Libraries/LibCore/TempFile.h
+++ b/Userland/Libraries/LibCore/TempFile.h
@@ -1,37 +1,41 @@
 /*
- * Copyright (c) 2020-2021, the SerenityOS developers.
+ * Copyright (c) 2020-2023, the SerenityOS developers.
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
 #pragma once
 
-#include <AK/DeprecatedString.h>
-#include <AK/Noncopyable.h>
+#include <AK/Error.h>
+#include <AK/NonnullOwnPtr.h>
+#include <AK/String.h>
 
 namespace Core {
 
 class TempFile {
-    AK_MAKE_NONCOPYABLE(TempFile);
-    AK_MAKE_NONMOVABLE(TempFile);
 
 public:
-    enum class Type {
-        File,
-        Directory
-    };
+    static ErrorOr<NonnullOwnPtr<TempFile>> create_temp_directory();
+    static ErrorOr<NonnullOwnPtr<TempFile>> create_temp_file();
 
-    static NonnullOwnPtr<TempFile> create(Type = Type::File);
     ~TempFile();
 
-    DeprecatedString path() const { return m_path; }
+    String const& path() const { return m_path; }
 
 private:
-    TempFile(Type);
-    static DeprecatedString create_temp(Type);
+    enum class Type {
+        Directory,
+        File
+    };
 
-    Type m_type { Type::File };
-    DeprecatedString m_path;
+    TempFile(Type type, String path)
+        : m_type(type)
+        , m_path(move(path))
+    {
+    }
+
+    Type m_type;
+    String m_path;
 };
 
 }