From 1ee808fae68ce1ebbe9eebd5924b87787317e617 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Tue, 13 Dec 2022 14:49:32 -0500 Subject: [PATCH] LibCore: Define and use a fallible, OS-independent getpwent(_r) wrapper Rather than maintaining a list of #ifdef guards to check systems that do not provide the reentrant version of getpwent, 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/Account.cpp | 45 +++++++++++--------------- Userland/Libraries/LibCore/System.cpp | 39 ++++++++++++++++++++++ Userland/Libraries/LibCore/System.h | 1 + 3 files changed, 58 insertions(+), 27 deletions(-) diff --git a/Userland/Libraries/LibCore/Account.cpp b/Userland/Libraries/LibCore/Account.cpp index e48a67541a..03808e089c 100644 --- a/Userland/Libraries/LibCore/Account.cpp +++ b/Userland/Libraries/LibCore/Account.cpp @@ -128,40 +128,31 @@ ErrorOr Account::from_uid(uid_t uid, [[maybe_unused]] Read options) ErrorOr> Account::all([[maybe_unused]] Read options) { Vector accounts; - -#ifndef AK_OS_MACOS - struct passwd pwd; - struct passwd* ptr = nullptr; char buffer[1024] = { 0 }; -#endif ScopeGuard pwent_guard([] { endpwent(); }); setpwent(); - errno = 0; -#ifndef AK_OS_MACOS - while (getpwent_r(&pwd, buffer, sizeof(buffer), &ptr) == 0 && ptr) { -#else - while (auto const* ptr = getpwent()) { -#endif + while (true) { + auto pwd = TRY(Core::System::getpwent({ buffer, sizeof(buffer) })); + if (!pwd.has_value()) + break; + spwd spwd = {}; #ifndef AK_OS_BSD_GENERIC ScopeGuard spent_guard([] { endspent(); }); if (options != Read::PasswdOnly) { - auto maybe_spwd = TRY(Core::System::getspnam({ ptr->pw_name, strlen(ptr->pw_name) })); + auto maybe_spwd = TRY(Core::System::getspnam({ pwd->pw_name, strlen(pwd->pw_name) })); if (!maybe_spwd.has_value()) return Error::from_string_literal("No shadow entry for user"); spwd = maybe_spwd.release_value(); } #endif - accounts.append({ *ptr, spwd, get_extra_gids(*ptr) }); + accounts.append({ *pwd, spwd, get_extra_gids(*pwd) }); } - if (errno) - return Error::from_errno(errno); - return accounts; } @@ -226,13 +217,17 @@ Account::Account(passwd const& pwd, spwd const& spwd, Vector extra_gids) ErrorOr Account::generate_passwd_file() const { StringBuilder builder; + char buffer[1024] = { 0 }; + ScopeGuard pwent_guard([] { endpwent(); }); setpwent(); - struct passwd* p; - errno = 0; - while ((p = getpwent())) { - if (p->pw_name == m_username) { + while (true) { + auto pwd = TRY(Core::System::getpwent({ buffer, sizeof(buffer) })); + if (!pwd.has_value()) + break; + + if (pwd->pw_name == m_username) { builder.appendff("{}:!:{}:{}:{}:{}:{}\n", m_username, m_uid, m_gid, @@ -242,15 +237,11 @@ ErrorOr Account::generate_passwd_file() const } else { builder.appendff("{}:!:{}:{}:{}:{}:{}\n", - p->pw_name, p->pw_uid, - p->pw_gid, p->pw_gecos, p->pw_dir, - p->pw_shell); + pwd->pw_name, pwd->pw_uid, + pwd->pw_gid, pwd->pw_gecos, pwd->pw_dir, + pwd->pw_shell); } } - endpwent(); - - if (errno) - return Error::from_errno(errno); return builder.to_deprecated_string(); } diff --git a/Userland/Libraries/LibCore/System.cpp b/Userland/Libraries/LibCore/System.cpp index 46b92231dd..faba7a654c 100644 --- a/Userland/Libraries/LibCore/System.cpp +++ b/Userland/Libraries/LibCore/System.cpp @@ -52,6 +52,40 @@ static int memfd_create(char const* name, unsigned int flags) } \ return success_value; +// clang-format off +template +concept SupportsReentrantGetpwent = requires(T passwd, T* ptr) +{ + getpwent_r(&passwd, 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 getpwent_r when it doesn't actually exist. +static ErrorOr> getpwent_impl(Span buffer) +{ + if constexpr (SupportsReentrantGetpwent) { + struct passwd passwd; + struct passwd* ptr = nullptr; + + extern int getpwent_r(struct passwd*, char*, size_t, struct passwd**); + auto result = getpwent_r(&passwd, buffer.data(), buffer.size(), &ptr); + + if (result == 0 && ptr) + return passwd; + if (result != 0 && result != ENOENT) + return Error::from_errno(result); + } else { + errno = 0; + if (auto const* passwd = ::getpwent()) + return *passwd; + if (errno) + return Error::from_errno(errno); + } + + return Optional {}; +} + namespace Core::System { #ifndef HOST_NAME_MAX @@ -627,6 +661,11 @@ ErrorOr chown(StringView pathname, uid_t uid, gid_t gid) #endif } +ErrorOr> getpwent(Span buffer) +{ + return getpwent_impl(buffer); +} + ErrorOr> getpwuid(uid_t uid) { errno = 0; diff --git a/Userland/Libraries/LibCore/System.h b/Userland/Libraries/LibCore/System.h index e7c673a344..c9287cab71 100644 --- a/Userland/Libraries/LibCore/System.h +++ b/Userland/Libraries/LibCore/System.h @@ -122,6 +122,7 @@ ErrorOr tcsetpgrp(int fd, pid_t pgrp); ErrorOr chmod(StringView pathname, mode_t mode); ErrorOr lchown(StringView pathname, uid_t uid, gid_t gid); ErrorOr chown(StringView pathname, uid_t uid, gid_t gid); +ErrorOr> getpwent(Span buffer); ErrorOr> getpwnam(StringView name); ErrorOr> getgrnam(StringView name); ErrorOr> getpwuid(uid_t);