From d09266237d07924f382fb193159e0b5504b18135 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Tue, 13 Dec 2022 15:08:58 -0500 Subject: [PATCH] LibCore: Define and use a fallible, OS-independent getgrent(_r) wrapper Rather than maintaining a list of #ifdef guards to check systems that do not provide the reentrant version of getgrent, we can use C++ concepts to let the compiler perform this check for us. While we're at it, we can also provide this wrapper as fallible to let the caller TRY calling it. --- Userland/Libraries/LibCore/Group.cpp | 52 +++++++++++---------------- Userland/Libraries/LibCore/System.cpp | 39 ++++++++++++++++++++ Userland/Libraries/LibCore/System.h | 1 + 3 files changed, 61 insertions(+), 31 deletions(-) diff --git a/Userland/Libraries/LibCore/Group.cpp b/Userland/Libraries/LibCore/Group.cpp index 3ebc090fe3..6dc3c172a3 100644 --- a/Userland/Libraries/LibCore/Group.cpp +++ b/Userland/Libraries/LibCore/Group.cpp @@ -19,32 +19,27 @@ namespace Core { ErrorOr Group::generate_group_file() const { StringBuilder builder; + char buffer[1024] = { 0 }; ScopeGuard grent_guard([] { endgrent(); }); setgrent(); - errno = 0; -#ifndef AK_OS_MACOS - struct group group; - struct group* gr = nullptr; - char buffer[1024] = { 0 }; - while (getgrent_r(&group, buffer, sizeof(buffer), &gr) == 0 && gr) { -#else - for (auto const* gr = getgrent(); gr; gr = getgrent()) { -#endif - if (gr->gr_name == m_name) + + while (true) { + auto group = TRY(Core::System::getgrent({ buffer, sizeof(buffer) })); + if (!group.has_value()) + break; + + if (group->gr_name == m_name) builder.appendff("{}:x:{}:{}\n", m_name, m_id, DeprecatedString::join(',', m_members)); else { Vector members; - for (size_t i = 0; gr->gr_mem[i]; ++i) - members.append(gr->gr_mem[i]); + for (size_t i = 0; group->gr_mem[i]; ++i) + members.append(group->gr_mem[i]); - builder.appendff("{}:x:{}:{}\n", gr->gr_name, gr->gr_gid, DeprecatedString::join(',', members)); + builder.appendff("{}:x:{}:{}\n", group->gr_name, group->gr_gid, DeprecatedString::join(',', members)); } } - if (errno) - return Error::from_errno(errno); - return builder.to_deprecated_string(); } @@ -123,28 +118,23 @@ ErrorOr Group::add_group(Group& group) ErrorOr> Group::all() { Vector groups; + char buffer[1024] = { 0 }; ScopeGuard grent_guard([] { endgrent(); }); setgrent(); - errno = 0; -#ifndef AK_OS_MACOS - struct group group; - struct group* gr = nullptr; - char buffer[1024] = { 0 }; - while (getgrent_r(&group, buffer, sizeof(buffer), &gr) == 0 && gr) { -#else - for (auto const* gr = getgrent(); gr; gr = getgrent()) { -#endif + + while (true) { + auto group = TRY(Core::System::getgrent({ buffer, sizeof(buffer) })); + if (!group.has_value()) + break; + Vector members; - for (size_t i = 0; gr->gr_mem[i]; ++i) - members.append(gr->gr_mem[i]); + for (size_t i = 0; group->gr_mem[i]; ++i) + members.append(group->gr_mem[i]); - groups.append({ gr->gr_name, gr->gr_gid, members }); + groups.append({ group->gr_name, group->gr_gid, members }); } - if (errno) - return Error::from_errno(errno); - return groups; } diff --git a/Userland/Libraries/LibCore/System.cpp b/Userland/Libraries/LibCore/System.cpp index faba7a654c..8126cde76d 100644 --- a/Userland/Libraries/LibCore/System.cpp +++ b/Userland/Libraries/LibCore/System.cpp @@ -86,6 +86,40 @@ static ErrorOr> getpwent_impl(Span buffer) return Optional {}; } +// clang-format off +template +concept SupportsReentrantGetgrent = requires(T group, T* ptr) +{ + getgrent_r(&group, nullptr, 0, &ptr); +}; +// clang-format on + +// Note: This has to be in the global namespace for the extern declaration to trick the compiler +// into finding a declaration of getgrent_r when it doesn't actually exist. +static ErrorOr> getgrent_impl(Span buffer) +{ + if constexpr (SupportsReentrantGetgrent) { + struct group group; + struct group* ptr = nullptr; + + extern int getgrent_r(struct group*, char*, size_t, struct group**); + auto result = getgrent_r(&group, buffer.data(), buffer.size(), &ptr); + + if (result == 0 && ptr) + return group; + if (result != 0 && result != ENOENT) + return Error::from_errno(result); + } else { + errno = 0; + if (auto const* group = ::getgrent()) + return *group; + if (errno) + return Error::from_errno(errno); + } + + return Optional {}; +} + namespace Core::System { #ifndef HOST_NAME_MAX @@ -676,6 +710,11 @@ ErrorOr> getpwuid(uid_t uid) return Optional {}; } +ErrorOr> getgrent(Span buffer) +{ + return getgrent_impl(buffer); +} + ErrorOr> getgrgid(gid_t gid) { errno = 0; diff --git a/Userland/Libraries/LibCore/System.h b/Userland/Libraries/LibCore/System.h index c9287cab71..bcc6bb4dbe 100644 --- a/Userland/Libraries/LibCore/System.h +++ b/Userland/Libraries/LibCore/System.h @@ -126,6 +126,7 @@ ErrorOr> getpwent(Span buffer); ErrorOr> getpwnam(StringView name); ErrorOr> getgrnam(StringView name); ErrorOr> getpwuid(uid_t); +ErrorOr> getgrent(Span buffer); ErrorOr> getgrgid(gid_t); ErrorOr clock_settime(clockid_t clock_id, struct timespec* ts); ErrorOr posix_spawn(StringView path, posix_spawn_file_actions_t const* file_actions, posix_spawnattr_t const* attr, char* const arguments[], char* const envp[]);