From 524baa29e866d6ee5ef2340c911d626c0b247ce8 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 17 Nov 2022 20:43:42 +0100 Subject: [PATCH] LibC+Tests: Simplify getpwuid_r() and getpwnam_r() and add tests These functions are now implemented in terms of getpwent_r() which allows us to remove two FIXMEs about global variable shenanigans. I'm also adding tests for both APIs. :^) --- Tests/LibC/CMakeLists.txt | 1 + Tests/LibC/TestPwd.cpp | 118 ++++++++++++++++++++++++++++++++ Userland/Libraries/LibC/pwd.cpp | 101 +++++---------------------- 3 files changed, 135 insertions(+), 85 deletions(-) create mode 100644 Tests/LibC/TestPwd.cpp diff --git a/Tests/LibC/CMakeLists.txt b/Tests/LibC/CMakeLists.txt index cd9ac9ebfe..1825ed23e4 100644 --- a/Tests/LibC/CMakeLists.txt +++ b/Tests/LibC/CMakeLists.txt @@ -20,6 +20,7 @@ set(TEST_SOURCES TestPThreadPriority.cpp TestPthreadSpinLocks.cpp TestPthreadRWLocks.cpp + TestPwd.cpp TestQsort.cpp TestRaise.cpp TestRealpath.cpp diff --git a/Tests/LibC/TestPwd.cpp b/Tests/LibC/TestPwd.cpp new file mode 100644 index 0000000000..0da3c586d4 --- /dev/null +++ b/Tests/LibC/TestPwd.cpp @@ -0,0 +1,118 @@ +/* + * Copyright (c) 2022, Andreas Kling + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include + +#include +#include + +struct PasswdEntry { + String name; + uid_t uid {}; +}; + +static Vector get_all_passwd_entries() +{ + Vector entries; + setpwent(); + while (auto* pwd = getpwent()) { + entries.append(PasswdEntry { + .name = pwd->pw_name, + .uid = pwd->pw_uid, + }); + } + return entries; +} + +TEST_CASE(getpwuid_r) +{ + // Verify that all known passwd entries can be found with getpwuid_r() + for (auto const& entry : get_all_passwd_entries()) { + struct passwd pwd_buffer = {}; + char buffer[4096] = {}; + struct passwd* result = nullptr; + auto rc = getpwuid_r(entry.uid, &pwd_buffer, buffer, sizeof(buffer), &result); + EXPECT_EQ(rc, 0); + EXPECT_EQ(entry.uid, result->pw_uid); + EXPECT_EQ(entry.name, result->pw_name); + } + + // Verify that a bogus UID can't be found with getpwuid_r() + { + struct passwd pwd_buffer = {}; + char buffer[4096] = {}; + struct passwd* result = nullptr; + auto rc = getpwuid_r(99991999, &pwd_buffer, buffer, sizeof(buffer), &result); + EXPECT_EQ(rc, ENOENT); + EXPECT_EQ(result, nullptr); + } + + // Verify that two calls to getpwuid_r() don't clobber each other. + { + struct passwd pwd_buffer1 = {}; + char buffer1[4096] = {}; + struct passwd* result1 = nullptr; + auto rc1 = getpwuid_r(0, &pwd_buffer1, buffer1, sizeof(buffer1), &result1); + EXPECT_EQ(rc1, 0); + EXPECT_NE(result1, nullptr); + EXPECT_EQ(result1, &pwd_buffer1); + + struct passwd pwd_buffer2 = {}; + char buffer2[4096] = {}; + struct passwd* result2 = nullptr; + auto rc2 = getpwuid_r(0, &pwd_buffer2, buffer2, sizeof(buffer2), &result2); + EXPECT_EQ(rc2, 0); + EXPECT_NE(result2, nullptr); + EXPECT_EQ(result2, &pwd_buffer2); + + EXPECT_NE(result1, result2); + } +} + +TEST_CASE(getpwnam_r) +{ + // Verify that all known passwd entries can be found with getpwnam_r() + for (auto const& entry : get_all_passwd_entries()) { + struct passwd pwd_buffer = {}; + char buffer[4096] = {}; + struct passwd* result = nullptr; + auto rc = getpwnam_r(entry.name.characters(), &pwd_buffer, buffer, sizeof(buffer), &result); + EXPECT_EQ(rc, 0); + EXPECT_EQ(entry.uid, result->pw_uid); + EXPECT_EQ(entry.name, result->pw_name); + } + + // Verify that a bogus name can't be found with getpwnam_r() + { + struct passwd pwd_buffer = {}; + char buffer[4096] = {}; + struct passwd* result = nullptr; + auto rc = getpwnam_r("99991999", &pwd_buffer, buffer, sizeof(buffer), &result); + EXPECT_EQ(rc, ENOENT); + EXPECT_EQ(result, nullptr); + } + + // Verify that two calls to getpwnam_r() don't clobber each other. + { + struct passwd pwd_buffer1 = {}; + char buffer1[4096] = {}; + struct passwd* result1 = nullptr; + auto rc1 = getpwnam_r("root", &pwd_buffer1, buffer1, sizeof(buffer1), &result1); + EXPECT_EQ(rc1, 0); + EXPECT_NE(result1, nullptr); + EXPECT_EQ(result1, &pwd_buffer1); + + struct passwd pwd_buffer2 = {}; + char buffer2[4096] = {}; + struct passwd* result2 = nullptr; + auto rc2 = getpwnam_r("root", &pwd_buffer2, buffer2, sizeof(buffer2), &result2); + EXPECT_EQ(rc2, 0); + EXPECT_NE(result2, nullptr); + EXPECT_EQ(result2, &pwd_buffer2); + + EXPECT_NE(result1, result2); + } +} diff --git a/Userland/Libraries/LibC/pwd.cpp b/Userland/Libraries/LibC/pwd.cpp index 688d3f306c..12aed49973 100644 --- a/Userland/Libraries/LibC/pwd.cpp +++ b/Userland/Libraries/LibC/pwd.cpp @@ -20,12 +20,6 @@ extern "C" { static FILE* s_stream = nullptr; static unsigned s_line_number = 0; -static String s_name; -static String s_passwd; -static String s_gecos; -static String s_dir; -static String s_shell; - void setpwent() { s_line_number = 0; @@ -163,91 +157,28 @@ int getpwent_r(struct passwd* passwd_buf, char* buffer, size_t buffer_size, stru } } -static void construct_pwd(struct passwd* pwd, char* buf, struct passwd** result) +// https://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwnam.html +int getpwnam_r(char const* name, struct passwd* pwd, char* buffer, size_t bufsize, struct passwd** result) { - auto* buf_name = &buf[0]; - auto* buf_passwd = &buf[s_name.length() + 1]; - auto* buf_gecos = &buf[s_name.length() + 1 + s_gecos.length() + 1]; - auto* buf_dir = &buf[s_gecos.length() + 1 + s_name.length() + 1 + s_gecos.length() + 1]; - auto* buf_shell = &buf[s_dir.length() + 1 + s_gecos.length() + 1 + s_name.length() + 1 + s_gecos.length() + 1]; - - bool ok = true; - ok = ok && s_name.copy_characters_to_buffer(buf_name, s_name.length() + 1); - ok = ok && s_passwd.copy_characters_to_buffer(buf_passwd, s_passwd.length() + 1); - ok = ok && s_gecos.copy_characters_to_buffer(buf_gecos, s_gecos.length() + 1); - ok = ok && s_dir.copy_characters_to_buffer(buf_dir, s_dir.length() + 1); - ok = ok && s_shell.copy_characters_to_buffer(buf_shell, s_shell.length() + 1); - - VERIFY(ok); - - *result = pwd; - pwd->pw_name = buf_name; - pwd->pw_passwd = buf_passwd; - pwd->pw_gecos = buf_gecos; - pwd->pw_dir = buf_dir; - pwd->pw_shell = buf_shell; + setpwent(); + for (;;) { + if (auto rc = getpwent_r(pwd, buffer, bufsize, result); rc != 0) + return rc; + if (strcmp(pwd->pw_name, name) == 0) + return 0; + } } -int getpwnam_r(char const* name, struct passwd* pwd, char* buf, size_t buflen, struct passwd** result) +// https://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid.html +int getpwuid_r(uid_t uid, struct passwd* pwd, char* buffer, size_t bufsize, struct passwd** result) { - // FIXME: This is a HACK! - TemporaryChange name_change { s_name, {} }; - TemporaryChange passwd_change { s_passwd, {} }; - TemporaryChange gecos_change { s_gecos, {} }; - TemporaryChange dir_change { s_dir, {} }; - TemporaryChange shell_change { s_shell, {} }; - setpwent(); - bool found = false; - while (auto* pw = getpwent()) { - if (!strcmp(pw->pw_name, name)) { - found = true; - break; - } + for (;;) { + if (auto rc = getpwent_r(pwd, buffer, bufsize, result); rc != 0) + return rc; + if (pwd->pw_uid == uid) + return 0; } - - if (!found) { - *result = nullptr; - return 0; - } - - auto const total_buffer_length = s_name.length() + s_passwd.length() + s_gecos.length() + s_dir.length() + s_shell.length() + 5; - if (buflen < total_buffer_length) - return ERANGE; - - construct_pwd(pwd, buf, result); - return 0; -} - -int getpwuid_r(uid_t uid, struct passwd* pwd, char* buf, size_t buflen, struct passwd** result) -{ - // FIXME: This is a HACK! - TemporaryChange name_change { s_name, {} }; - TemporaryChange passwd_change { s_passwd, {} }; - TemporaryChange gecos_change { s_gecos, {} }; - TemporaryChange dir_change { s_dir, {} }; - TemporaryChange shell_change { s_shell, {} }; - - setpwent(); - bool found = false; - while (auto* pw = getpwent()) { - if (pw->pw_uid == uid) { - found = true; - break; - } - } - - if (!found) { - *result = nullptr; - return 0; - } - - auto const total_buffer_length = s_name.length() + s_passwd.length() + s_gecos.length() + s_dir.length() + s_shell.length() + 5; - if (buflen < total_buffer_length) - return ERANGE; - - construct_pwd(pwd, buf, result); - return 0; } int putpwent(const struct passwd* p, FILE* stream)