LibCore: Add CDirIterator, and use it in everything rather than readdir

Author: https://github.com/rburchell
Commit: https://github.com/SerenityOS/serenity/commit/9d2b08e06eb
Pull-request: https://github.com/SerenityOS/serenity/pull/123
Reviewed-by: https://github.com/awesomekling ✅

LibCore/CDirIterator.cpp

@@ -0,0 +1,68 @@
+#include "CDirIterator.h"
+#include <cerrno>
+
+CDirIterator::CDirIterator(const String& path, Flags flags)
+    : m_flags(flags)
+{
+    m_dir = opendir(path.characters());
+    if (m_dir == nullptr) {
+        m_error = errno;
+    }
+}
+
+CDirIterator::~CDirIterator()
+{
+    if (m_dir != nullptr) {
+        closedir(m_dir);
+        m_dir = nullptr;
+    }
+}
+
+bool CDirIterator::advance_next()
+{
+    if (m_dir == nullptr)
+        return false;
+
+    bool keep_advancing = true;
+    while (keep_advancing) {
+        errno = 0;
+        auto* de = readdir(m_dir);
+        if (de) {
+            m_next = de->d_name;
+        } else {
+            m_error = errno;
+            m_next = String();
+        }
+
+        if (m_next.is_null()) {
+            keep_advancing = false;
+        } else if (m_flags & Flags::SkipDots) {
+            if (m_next.length() < 1 || m_next[0] != '.') {
+                keep_advancing = false;
+            }
+        } else {
+            keep_advancing = false;
+        }
+    }
+
+    return m_next.length() > 0;
+}
+
+bool CDirIterator::has_next()
+{
+    if (!m_next.is_null())
+        return true;
+
+    return advance_next();
+}
+
+String CDirIterator::next_path()
+{
+    if (m_next.is_null())
+        advance_next();
+
+    auto tmp = m_next;
+    m_next = String();
+    return tmp;
+}
+

LibCore/CDirIterator.h

@@ -0,0 +1,30 @@
+#pragma once
+
+#include <dirent.h>
+#include <AK/AKString.h>
+
+class CDirIterator {
+public:
+    enum Flags {
+        NoFlags = 0x0,
+        SkipDots = 0x1,
+    };
+
+    CDirIterator(const String& path, Flags = Flags::NoFlags);
+    ~CDirIterator();
+
+    bool has_error() const { return m_error != 0; }
+    int error() const { return m_error; }
+    const char* error_string() const { return strerror(m_error); }
+    bool has_next();
+    String next_path();
+
+private:
+    DIR* m_dir = nullptr;
+    int m_error = 0;
+    String m_next;
+    int m_flags;
+
+    bool advance_next();
+};
+

LibCore/Makefile

@@ -18,7 +18,8 @@ OBJS = \
     CEventLoop.o \
     CConfigFile.o \
     CEvent.o \
-    CProcessStatisticsReader.o
+    CProcessStatisticsReader.o \
+    CDirIterator.o
 
 LIBRARY = libcore.a
 DEFINES += -DUSERLAND

LibGUI/GDirectoryModel.cpp

@@ -9,6 +9,7 @@
 #include <SharedGraphics/GraphicsBitmap.h>
 #include <LibGUI/GPainter.h>
 #include <LibCore/CLock.h>
+#include <LibCore/CDirIterator.h>
 
 static CLockable<HashMap<String, RetainPtr<GraphicsBitmap>>>& thumbnail_cache()
 {
@@ -235,22 +236,23 @@ GVariant GDirectoryModel::data(const GModelIndex& index, Role role) const
 
 void GDirectoryModel::update()
 {
-    DIR* dirp = opendir(m_path.characters());
-    if (!dirp) {
-        perror("opendir");
+    CDirIterator di(m_path, CDirIterator::SkipDots);
+    if (di.has_error()) {
+        fprintf(stderr, "CDirIterator: %s\n", di.error_string());
         exit(1);
     }
+
     m_directories.clear();
     m_files.clear();
 
     m_bytes_in_files = 0;
-    while (auto* de = readdir(dirp)) {
+    while (di.has_next()) {
+        String name = di.next_path();
         Entry entry;
-        entry.name = de->d_name;
-        if (entry.name == "." || entry.name == "..")
-            continue;
+        entry.name = name;
+
         struct stat st;
-        int rc = lstat(String::format("%s/%s", m_path.characters(), de->d_name).characters(), &st);
+        int rc = lstat(String::format("%s/%s", m_path.characters(), name.characters()).characters(), &st);
         if (rc < 0) {
             perror("lstat");
             continue;
@@ -266,7 +268,6 @@ void GDirectoryModel::update()
         if (S_ISREG(entry.mode))
             m_bytes_in_files += st.st_size;
     }
-    closedir(dirp);
 
     did_update();
 }

LibGUI/GFileSystemModel.cpp

@@ -1,4 +1,5 @@
 #include <LibGUI/GFileSystemModel.h>
+#include <LibCore/CDirIterator.h>
 #include <AK/FileSystemPath.h>
 #include <AK/StringBuilder.h>
 #include <sys/stat.h>
@@ -33,15 +34,16 @@ struct GFileSystemModel::Node {
         has_traversed = true;
 
         auto full_path = this->full_path(model);
-        DIR* dirp = opendir(full_path.characters());
-        if (!dirp)
+        CDirIterator di(full_path, CDirIterator::SkipDots);
+        if (di.has_error()) {
+            fprintf(stderr, "CDirIterator: %s\n", di.error_string());
             return;
+        }
 
-        while (auto* de = readdir(dirp)) {
-            if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
-                continue;
+        while (di.has_next()) {
+            String name = di.next_path();
             struct stat st;
-            int rc = lstat(String::format("%s/%s", full_path.characters(), de->d_name).characters(), &st);
+            int rc = lstat(String::format("%s/%s", full_path.characters(), name.characters()).characters(), &st);
             if (rc < 0) {
                 perror("lstat");
                 continue;
@@ -49,13 +51,11 @@ struct GFileSystemModel::Node {
             if (model.m_mode == DirectoriesOnly && !S_ISDIR(st.st_mode))
                 continue;
             auto* child = new Node;
-            child->name = de->d_name;
+            child->name = name;
             child->type = S_ISDIR(st.st_mode) ? Node::Type::Directory : Node::Type::File;
             child->parent = this;
             children.append(child);
         }
-
-        closedir(dirp);
     }
 
     void reify_if_needed(const GFileSystemModel& model)

LibGUI/GFontDatabase.cpp

@@ -1,4 +1,5 @@
 #include <LibGUI/GFontDatabase.h>
+#include <LibCore/CDirIterator.h>
 #include <SharedGraphics/Font.h>
 #include <dirent.h>
 #include <stdio.h>
@@ -15,15 +16,14 @@ GFontDatabase& GFontDatabase::the()
 
 GFontDatabase::GFontDatabase()
 {
-    DIR* dirp = opendir("/res/fonts");
-    if (!dirp) {
-        perror("opendir");
+    CDirIterator di("/res/fonts", CDirIterator::SkipDots);
+    if (di.has_error()) {
+        fprintf(stderr, "CDirIterator: %s\n", di.error_string());
         exit(1);
     }
-    while (auto* de = readdir(dirp)) {
-        if (de->d_name[0] == '.')
-            continue;
-        auto path = String::format("/res/fonts/%s", de->d_name);
+    while (di.has_next()) {
+        String name = di.next_path();
+        auto path = String::format("/res/fonts/%s", name.characters());
         if (auto font = Font::load_from_file(path)) {
             Metadata metadata;
             metadata.path = path;
@@ -32,7 +32,6 @@ GFontDatabase::GFontDatabase()
             m_name_to_metadata.set(font->name(), move(metadata));
         }
     }
-    closedir(dirp);
 }
 
 GFontDatabase::~GFontDatabase()

Userland/ls.cpp

@@ -10,6 +10,7 @@
 #include <sys/stat.h>
 #include <AK/AKString.h>
 #include <AK/Vector.h>
+#include <LibCore/CDirIterator.h>
 
 static int do_file_system_object_long(const char* path);
 static int do_file_system_object_short(const char* path);
@@ -221,9 +222,9 @@ int do_file_system_object_short(const char* path)
     int columns;
     get_geometry(rows, columns);
 
-    DIR* dirp = opendir(path);
-    if (!dirp) {
-        if (errno == ENOTDIR) {
+    CDirIterator di(path, !flag_show_dotfiles ? CDirIterator::SkipDots : CDirIterator::Flags::NoFlags);
+    if (di.has_error()) {
+        if (di.error() == ENOTDIR) {
             int nprinted;
             bool status = print_filesystem_object_short(path, path, &nprinted);
             printf("\n");
@@ -231,20 +232,18 @@ int do_file_system_object_short(const char* path)
                 return 0;
             return 2;
         }
-        perror("opendir");
+        fprintf(stderr, "CDirIterator: %s\n", di.error_string());
         return 1;
     }
 
     Vector<String, 1024> names;
     int longest_name = 0;
-    while (auto* de = readdir(dirp)) {
-        if (de->d_name[0] == '.' && !flag_show_dotfiles)
-            continue;
-        names.append(de->d_name);
+    while (di.has_next()) {
+        String name = di.next_path();
+        names.append(name);
         if (names.last().length() > longest_name)
-            longest_name = names.last().length();
+            longest_name = name.length();
     }
-    closedir(dirp);
 
     int printed_on_row = 0;
     int nprinted;

Userland/pape.cpp

@@ -10,22 +10,22 @@
 #include <AK/Vector.h>
 #include <AK/FileSystemPath.h>
 #include <LibCore/CArgsParser.h>
+#include <LibCore/CDirIterator.h>
 #include <LibGUI/GDesktop.h>
 #include <LibGUI/GApplication.h>
 
 static int handle_show_all()
 {
-    DIR* dirp = opendir("/res/wallpapers");
-    if (!dirp) {
-        perror("opendir");
+    CDirIterator di("/res/wallpapers", CDirIterator::SkipDots);
+    if (di.has_error()) {
+        fprintf(stderr, "CDirIterator: %s\n", di.error_string());
         return 1;
     }
-    while (auto* de = readdir(dirp)) {
-        if (de->d_name[0] == '.')
-            continue;
-        printf("%s\n", de->d_name);
+
+    while (di.has_next()) {
+        String name = di.next_path();
+        printf("%s\n", name.characters());
     }
-    closedir(dirp);
     return 0;
 }
 

Userland/sysctl.cpp

@@ -8,6 +8,7 @@
 #include <AK/StringBuilder.h>
 #include <AK/Vector.h>
 #include <LibCore/CArgsParser.h>
+#include <LibCore/CDirIterator.h>
 
 static String read_var(const String& name)
 {
@@ -52,17 +53,16 @@ static void write_var(const String& name, const String& value)
 
 static int handle_show_all()
 {
-    DIR* dirp = opendir("/proc/sys");
-    if (!dirp) {
-        perror("opendir");
+    CDirIterator di("/proc/sys", CDirIterator::SkipDots);
+    if (di.has_error()) {
+        fprintf(stderr, "CDirIterator: %s\n", di.error_string());
         return 1;
     }
-    char pathbuf[PATH_MAX];
 
-    while (auto* de = readdir(dirp)) {
-        if (de->d_name[0] == '.')
-            continue;
-        sprintf(pathbuf, "/proc/sys/%s", de->d_name);
+    char pathbuf[PATH_MAX];
+    while (di.has_next()) {
+        String name = di.next_path();
+        sprintf(pathbuf, "/proc/sys/%s", name.characters());
         int fd = open(pathbuf, O_RDONLY);
         if (fd < 0) {
             perror("open");
@@ -76,11 +76,10 @@ static int handle_show_all()
             continue;
         }
         buffer[nread] = '\0';
-        printf("%s = %s", de->d_name, buffer);
+        printf("%s = %s", name.characters(), buffer);
         if (nread && buffer[nread - 1] != '\n')
             printf("\n");
     }
-    closedir(dirp);
     return 0;
 }