From d771a7cde39c39e094eb37974e987aed341c2a4c Mon Sep 17 00:00:00 2001
From: Elliott Hughes <enh@google.com>
Date: Thu, 28 Jul 2022 17:52:46 +0000
Subject: [PATCH] Simplify the implementation of get_nproc().

It came up on the musl mailing list that there's not actually any need
to iterate over the directory entries:

https://www.openwall.com/lists/musl/2022/07/27/1

This lets us reuse the code for "online" processors in the
implementation of "configured" processors. The question of whether
"configured" should correspond to Linux's "possible" or "present" isn't
obvious to me, but the distinction seems unlikely to matter on mobile
devices anyway, and that's a trivial change should it ever be needed.
Plus the motivating argument from the person who brought this up was
that callers asking for "configured" processors are probably asking for
an upper bound, which sounds convincing to me.

Test: treehugger
Change-Id: I0d4e13538dc6b09a6dba520d9ac24f436906f7c0
---
 libc/bionic/sysinfo.cpp | 44 +++++++++++++----------------------------
 tests/unistd_test.cpp   |  4 ++++
 2 files changed, 18 insertions(+), 30 deletions(-)

diff --git a/libc/bionic/sysinfo.cpp b/libc/bionic/sysinfo.cpp
index 7ab8e9eda..897ef303b 100644
--- a/libc/bionic/sysinfo.cpp
+++ b/libc/bionic/sysinfo.cpp
@@ -36,39 +36,13 @@
 #include "private/get_cpu_count_from_string.h"
 #include "private/ScopedReaddir.h"
 
-static bool __matches_cpuN(const char* s) {
-  // The %c trick is to ensure that we have the anchored match "^cpu[0-9]+$".
-  // We can't use %*c because the return value is how many were *assigned*.
-  unsigned cpu;
-  char unused;
-  return (sscanf(s, "cpu%u%c", &cpu, &unused) == 1);
-}
-
-int get_nprocs_conf() {
-  // On x86 kernels you can use /proc/cpuinfo for this, but on ARM kernels offline CPUs disappear
-  // from there. This method works on both.
-  ScopedReaddir reader("/sys/devices/system/cpu");
-  if (reader.IsBad()) {
-    return 1;
-  }
-
-  int result = 0;
-  dirent* entry;
-  while ((entry = reader.ReadEntry()) != nullptr) {
-    if (entry->d_type == DT_DIR && __matches_cpuN(entry->d_name)) {
-      ++result;
-    }
-  }
-  return result;
-}
-
-int get_nprocs() {
+int __get_cpu_count(const char* sys_file) {
   int cpu_count = 1;
-  FILE* fp = fopen("/sys/devices/system/cpu/online", "re");
+  FILE* fp = fopen(sys_file, "re");
   if (fp != nullptr) {
     char* line = nullptr;
-    size_t len = 0;
-    if (getline(&line, &len, fp) != -1) {
+    size_t allocated_size = 0;
+    if (getline(&line, &allocated_size, fp) != -1) {
       cpu_count = GetCpuCountFromString(line);
       free(line);
     }
@@ -77,6 +51,16 @@ int get_nprocs() {
   return cpu_count;
 }
 
+int get_nprocs_conf() {
+  // It's unclear to me whether this is intended to be "possible" or "present",
+  // but on mobile they're unlikely to differ.
+  return __get_cpu_count("/sys/devices/system/cpu/possible");
+}
+
+int get_nprocs() {
+  return __get_cpu_count("/sys/devices/system/cpu/online");
+}
+
 long get_phys_pages() {
   struct sysinfo si;
   sysinfo(&si);
diff --git a/tests/unistd_test.cpp b/tests/unistd_test.cpp
index 02da5851d..5fce5b8bd 100644
--- a/tests/unistd_test.cpp
+++ b/tests/unistd_test.cpp
@@ -1103,6 +1103,10 @@ TEST(UNISTD_TEST, get_cpu_count_from_string) {
   ASSERT_EQ(4, GetCpuCountFromString("0, 1-2, 4\n"));
 }
 
+TEST(UNISTD_TEST, sysconf_SC_NPROCESSORS_make_sense) {
+  ASSERT_LE(sysconf(_SC_NPROCESSORS_ONLN), sysconf(_SC_NPROCESSORS_CONF));
+}
+
 TEST(UNISTD_TEST, sysconf_SC_NPROCESSORS_ONLN) {
   std::string line;
   ASSERT_TRUE(android::base::ReadFileToString("/sys/devices/system/cpu/online", &line));