Remove some globals from system_properties

pa_size should be static to prop_area, so make it so.

__system_property_area__ was reused for various purposes, but
realistically is a deprecated symbol and this finally separates us
from it.

Bug: 36001741
Test: boot bullhead, system property unit tests
Change-Id: I39663cc3b613093fa4c728b21d8ba58754f8e105
diff --git a/libc/system_properties/context_node.cpp b/libc/system_properties/context_node.cpp
index 440b865..13cef75 100644
--- a/libc/system_properties/context_node.cpp
+++ b/libc/system_properties/context_node.cpp
@@ -28,12 +28,11 @@
 
 #include "context_node.h"
 
-#include <sys/mman.h>
 #include <unistd.h>
 
 #include <async_safe/log.h>
 
-#include "system_property_globals.h"
+#include "property_filename.h"
 
 // pthread_mutex_lock() calls into system_properties in the case of contention.
 // This creates a risk of dead lock if any system_properties functions
@@ -96,10 +95,5 @@
 }
 
 void ContextNode::Unmap() {
-  if (!pa_) {
-    return;
-  }
-
-  munmap(pa_, pa_size);
-  pa_ = nullptr;
+  prop_area::unmap_prop_area(&pa_);
 }
diff --git a/libc/system_properties/contexts.h b/libc/system_properties/contexts.h
index 9910ed1..5df9d96 100644
--- a/libc/system_properties/contexts.h
+++ b/libc/system_properties/contexts.h
@@ -39,6 +39,7 @@
 
   virtual bool Initialize(bool writable) = 0;
   virtual prop_area* GetPropAreaForName(const char* name) = 0;
+  virtual prop_area* GetSerialPropArea() = 0;
   virtual void ForEach(void (*propfn)(const prop_info* pi, void* cookie), void* cookie) = 0;
   virtual void ResetAccess() = 0;
   virtual void FreeAndUnmap() = 0;
diff --git a/libc/system_properties/contexts_pre_split.h b/libc/system_properties/contexts_pre_split.h
index 7e83a37..bbc8529 100644
--- a/libc/system_properties/contexts_pre_split.h
+++ b/libc/system_properties/contexts_pre_split.h
@@ -29,12 +29,10 @@
 #ifndef SYSTEM_PROPERTIES_CONTEXTS_PRE_SPLIT_H
 #define SYSTEM_PROPERTIES_CONTEXTS_PRE_SPLIT_H
 
-#include <sys/mman.h>
-
 #include "contexts.h"
 #include "prop_area.h"
 #include "prop_info.h"
-#include "system_property_globals.h"
+#include "property_filename.h"
 
 class ContextsPreSplit : public Contexts {
  public:
@@ -43,16 +41,20 @@
 
   // We'll never initialize this legacy option as writable, so don't even check the arg.
   virtual bool Initialize(bool) override {
-    __system_property_area__ = prop_area::map_prop_area(property_filename);
-    return __system_property_area__ != nullptr;
+    pre_split_prop_area_ = prop_area::map_prop_area(property_filename);
+    return pre_split_prop_area_ != nullptr;
   }
 
   virtual prop_area* GetPropAreaForName(const char*) override {
-    return __system_property_area__;
+    return pre_split_prop_area_;
+  }
+
+  virtual prop_area* GetSerialPropArea() override {
+    return pre_split_prop_area_;
   }
 
   virtual void ForEach(void (*propfn)(const prop_info* pi, void* cookie), void* cookie) override {
-    __system_property_area__->foreach (propfn, cookie);
+    pre_split_prop_area_->foreach (propfn, cookie);
   }
 
   // This is a no-op for pre-split properties as there is only one property file and it is
@@ -61,11 +63,11 @@
   }
 
   virtual void FreeAndUnmap() override {
-    if (__system_property_area__ != nullptr) {
-      munmap(__system_property_area__, pa_size);
-      __system_property_area__ = nullptr;
-    }
+    prop_area::unmap_prop_area(&pre_split_prop_area_);
   }
+
+ private:
+  prop_area* pre_split_prop_area_ = nullptr;
 };
 
 #endif
diff --git a/libc/system_properties/contexts_split.cpp b/libc/system_properties/contexts_split.cpp
index 964a1e4..77f2069 100644
--- a/libc/system_properties/contexts_split.cpp
+++ b/libc/system_properties/contexts_split.cpp
@@ -31,13 +31,12 @@
 #include <ctype.h>
 #include <stdlib.h>
 #include <string.h>
-#include <sys/mman.h>
 #include <sys/stat.h>
 
 #include <async_safe/log.h>
 
 #include "context_node.h"
-#include "system_property_globals.h"
+#include "property_filename.h"
 
 class ContextListNode : public ContextNode {
  public:
@@ -193,22 +192,22 @@
   return items;
 }
 
-static bool MapSystemPropertyArea(bool access_rw, bool* fsetxattr_failed) {
+bool ContextsSplit::MapSerialPropertyArea(bool access_rw, bool* fsetxattr_failed) {
   char filename[PROP_FILENAME_MAX];
   int len = async_safe_format_buffer(filename, sizeof(filename), "%s/properties_serial",
                                      property_filename);
   if (len < 0 || len > PROP_FILENAME_MAX) {
-    __system_property_area__ = nullptr;
+    serial_prop_area_ = nullptr;
     return false;
   }
 
   if (access_rw) {
-    __system_property_area__ =
+    serial_prop_area_ =
         prop_area::map_prop_area_rw(filename, "u:object_r:properties_serial:s0", fsetxattr_failed);
   } else {
-    __system_property_area__ = prop_area::map_prop_area(filename);
+    serial_prop_area_ = prop_area::map_prop_area(filename);
   }
-  return __system_property_area__;
+  return serial_prop_area_;
 }
 
 bool ContextsSplit::InitializePropertiesFromFile(const char* filename) {
@@ -300,14 +299,14 @@
         open_failed = true;
       }
     });
-    if (open_failed || !MapSystemPropertyArea(true, &fsetxattr_failed)) {
+    if (open_failed || !MapSerialPropertyArea(true, &fsetxattr_failed)) {
       FreeAndUnmap();
       return false;
     }
 
     return !fsetxattr_failed;
   } else {
-    if (!MapSystemPropertyArea(false, nullptr)) {
+    if (!MapSerialPropertyArea(false, nullptr)) {
       FreeAndUnmap();
       return false;
     }
@@ -348,8 +347,5 @@
 void ContextsSplit::FreeAndUnmap() {
   ListFree(&prefixes_);
   ListFree(&contexts_);
-  if (__system_property_area__) {
-    munmap(__system_property_area__, pa_size);
-    __system_property_area__ = nullptr;
-  }
+  prop_area::unmap_prop_area(&serial_prop_area_);
 }
diff --git a/libc/system_properties/contexts_split.h b/libc/system_properties/contexts_split.h
index fced33c..f98eb44 100644
--- a/libc/system_properties/contexts_split.h
+++ b/libc/system_properties/contexts_split.h
@@ -41,16 +41,21 @@
 
   virtual bool Initialize(bool writable) override;
   virtual prop_area* GetPropAreaForName(const char* name) override;
+  virtual prop_area* GetSerialPropArea() override {
+    return serial_prop_area_;
+  }
   virtual void ForEach(void (*propfn)(const prop_info* pi, void* cookie), void* cookie) override;
   virtual void ResetAccess() override;
   virtual void FreeAndUnmap() override;
 
  private:
+  bool MapSerialPropertyArea(bool access_rw, bool* fsetxattr_failed);
   bool InitializePropertiesFromFile(const char* filename);
   bool InitializeProperties();
 
   PrefixNode* prefixes_ = nullptr;
   ContextListNode* contexts_ = nullptr;
+  prop_area* serial_prop_area_ = nullptr;
 };
 
 #endif
diff --git a/libc/system_properties/prop_area.cpp b/libc/system_properties/prop_area.cpp
index f6c7fa1..e11f292 100644
--- a/libc/system_properties/prop_area.cpp
+++ b/libc/system_properties/prop_area.cpp
@@ -29,7 +29,6 @@
 #include <fcntl.h>
 #include <stdlib.h>
 #include <sys/cdefs.h>
-#include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/xattr.h>
@@ -40,13 +39,13 @@
 #include <async_safe/log.h>
 
 #include "prop_area.h"
-#include "system_property_globals.h"
 
 constexpr size_t PA_SIZE = 128 * 1024;
 constexpr uint32_t PROP_AREA_MAGIC = 0x504f5250;
 constexpr uint32_t PROP_AREA_VERSION = 0xfc6ed0ab;
 
-static size_t pa_data_size;
+size_t prop_area::pa_size_ = 0;
+size_t prop_area::pa_data_size_ = 0;
 
 prop_area* prop_area::map_prop_area_rw(const char* filename, const char* context,
                                        bool* fsetxattr_failed) {
@@ -89,10 +88,10 @@
     return nullptr;
   }
 
-  pa_size = PA_SIZE;
-  pa_data_size = pa_size - sizeof(prop_area);
+  pa_size_ = PA_SIZE;
+  pa_data_size_ = pa_size_ - sizeof(prop_area);
 
-  void* const memory_area = mmap(nullptr, pa_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+  void* const memory_area = mmap(nullptr, pa_size_, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
   if (memory_area == MAP_FAILED) {
     close(fd);
     return nullptr;
@@ -116,17 +115,17 @@
     return nullptr;
   }
 
-  pa_size = fd_stat.st_size;
-  pa_data_size = pa_size - sizeof(prop_area);
+  pa_size_ = fd_stat.st_size;
+  pa_data_size_ = pa_size_ - sizeof(prop_area);
 
-  void* const map_result = mmap(nullptr, pa_size, PROT_READ, MAP_SHARED, fd, 0);
+  void* const map_result = mmap(nullptr, pa_size_, PROT_READ, MAP_SHARED, fd, 0);
   if (map_result == MAP_FAILED) {
     return nullptr;
   }
 
   prop_area* pa = reinterpret_cast<prop_area*>(map_result);
   if ((pa->magic() != PROP_AREA_MAGIC) || (pa->version() != PROP_AREA_VERSION)) {
-    munmap(pa, pa_size);
+    munmap(pa, pa_size_);
     return nullptr;
   }
 
@@ -145,7 +144,7 @@
 
 void* prop_area::allocate_obj(const size_t size, uint_least32_t* const off) {
   const size_t aligned = __BIONIC_ALIGN(size, sizeof(uint_least32_t));
-  if (bytes_used_ + aligned > pa_data_size) {
+  if (bytes_used_ + aligned > pa_data_size_) {
     return nullptr;
   }
 
@@ -195,7 +194,7 @@
 }
 
 void* prop_area::to_prop_obj(uint_least32_t off) {
-  if (off > pa_data_size) return nullptr;
+  if (off > pa_data_size_) return nullptr;
 
   return (data_ + off);
 }
diff --git a/libc/system_properties/prop_area.h b/libc/system_properties/prop_area.h
index 6b74e10..10c1adb 100644
--- a/libc/system_properties/prop_area.h
+++ b/libc/system_properties/prop_area.h
@@ -29,6 +29,7 @@
 #include <stdatomic.h>
 #include <stdint.h>
 #include <string.h>
+#include <sys/mman.h>
 
 #include "private/bionic_macros.h"
 
@@ -94,6 +95,12 @@
   static prop_area* map_prop_area_rw(const char* filename, const char* context,
                                      bool* fsetxattr_failed);
   static prop_area* map_prop_area(const char* filename);
+  static void unmap_prop_area(prop_area** pa) {
+    if (*pa) {
+      munmap(*pa, pa_size_);
+      *pa = nullptr;
+    }
+  }
 
   prop_area(const uint32_t magic, const uint32_t version) : magic_(magic), version_(version) {
     atomic_init(&serial_, 0);
@@ -138,6 +145,13 @@
   bool foreach_property(prop_bt* const trie, void (*propfn)(const prop_info* pi, void* cookie),
                         void* cookie);
 
+  // The original design doesn't include pa_size or pa_data_size in the prop_area struct itself.
+  // Since we'll need to be backwards compatible with that design, we don't gain much by adding it
+  // now, especially since we don't have any plans to make different property areas different sizes,
+  // and thus we share these two variables among all instances.
+  static size_t pa_size_;
+  static size_t pa_data_size_;
+
   uint32_t bytes_used_;
   atomic_uint_least32_t serial_;
   uint32_t magic_;
diff --git a/libc/system_properties/system_property_globals.h b/libc/system_properties/property_filename.h
similarity index 85%
rename from libc/system_properties/system_property_globals.h
rename to libc/system_properties/property_filename.h
index e37a4ac..0f7bcf7 100644
--- a/libc/system_properties/system_property_globals.h
+++ b/libc/system_properties/property_filename.h
@@ -26,18 +26,12 @@
  * SUCH DAMAGE.
  */
 
-#ifndef SYSTEM_PROPERTIES_SYSTEM_PROPERTY_GLOBALS_H
-#define SYSTEM_PROPERTIES_SYSTEM_PROPERTY_GLOBALS_H
+#ifndef SYSTEM_PROPERTIES_PROPERTY_FILENAME_H
+#define SYSTEM_PROPERTIES_PROPERTY_FILENAME_H
 
-#include "prop_area.h"
-
-// These were essentially globals before the refactoring.
-
+// These are globals set by __system_property_set_filename().
+// There isn't huge benefit in refactoring them, so they're alone in this header.
 constexpr int PROP_FILENAME_MAX = 1024;
-
 extern char property_filename[PROP_FILENAME_MAX];
-extern size_t pa_size;
-
-extern prop_area* __system_property_area__;
 
 #endif
diff --git a/libc/system_properties/system_properties.cpp b/libc/system_properties/system_properties.cpp
index 5d13404..3f6601d 100644
--- a/libc/system_properties/system_properties.cpp
+++ b/libc/system_properties/system_properties.cpp
@@ -60,7 +60,7 @@
 #include "contexts_split.h"
 #include "prop_area.h"
 #include "prop_info.h"
-#include "system_property_globals.h"
+#include "property_filename.h"
 
 // We don't want to use new or malloc in properties (b/31659220), and since these classes are
 // small enough and don't have non-trivial constructors, it's easier to just statically declare
@@ -76,11 +76,11 @@
 static const char* kServiceVersionPropertyName = "ro.property_service.version";
 
 // This is public because it was exposed in the NDK. As of 2017-01, ~60 apps reference this symbol.
+// It is set to nullptr and never modified.
 __BIONIC_WEAK_VARIABLE_FOR_NATIVE_BRIDGE
 prop_area* __system_property_area__ = nullptr;
 
 char property_filename[PROP_FILENAME_MAX] = PROP_FILENAME;
-size_t pa_size;
 
 class PropertyServiceConnection {
  public:
@@ -314,10 +314,15 @@
 
 __BIONIC_WEAK_FOR_NATIVE_BRIDGE
 uint32_t __system_property_area_serial() {
-  prop_area* pa = __system_property_area__;
+  if (contexts == nullptr) {
+    return -1;
+  }
+
+  prop_area* pa = contexts->GetSerialPropArea();
   if (!pa) {
     return -1;
   }
+
   // Make sure this read fulfilled before __system_property_serial
   return atomic_load_explicit(pa->serial(), memory_order_acquire);
 }
@@ -521,8 +526,11 @@
     return -1;
   }
 
-  prop_area* pa = __system_property_area__;
+  if (contexts == nullptr) {
+    return -1;
+  }
 
+  prop_area* pa = contexts->GetSerialPropArea();
   if (!pa) {
     return -1;
   }
@@ -557,12 +565,16 @@
     return -1;
   }
 
-  if (__system_property_area__ == nullptr || contexts == nullptr) {
+  if (contexts == nullptr) {
+    return -1;
+  }
+
+  prop_area* serial_pa = contexts->GetSerialPropArea();
+  if (serial_pa == nullptr) {
     return -1;
   }
 
   prop_area* pa = contexts->GetPropAreaForName(name);
-
   if (!pa) {
     async_safe_format_log(ANDROID_LOG_ERROR, "libc", "Access denied adding property \"%s\"", name);
     return -1;
@@ -575,11 +587,10 @@
 
   // There is only a single mutator, but we want to make sure that
   // updates are visible to a reader waiting for the update.
-  atomic_store_explicit(
-      __system_property_area__->serial(),
-      atomic_load_explicit(__system_property_area__->serial(), memory_order_relaxed) + 1,
-      memory_order_release);
-  __futex_wake(__system_property_area__->serial(), INT32_MAX);
+  atomic_store_explicit(serial_pa->serial(),
+                        atomic_load_explicit(serial_pa->serial(), memory_order_relaxed) + 1,
+                        memory_order_release);
+  __futex_wake(serial_pa->serial(), INT32_MAX);
   return 0;
 }
 
@@ -607,8 +618,16 @@
   // Are we waiting on the global serial or a specific serial?
   atomic_uint_least32_t* serial_ptr;
   if (pi == nullptr) {
-    if (__system_property_area__ == nullptr) return -1;
-    serial_ptr = __system_property_area__->serial();
+    if (contexts == nullptr) {
+      return -1;
+    }
+
+    prop_area* serial_pa = contexts->GetSerialPropArea();
+    if (serial_pa == nullptr) {
+      return -1;
+    }
+
+    serial_ptr = serial_pa->serial();
   } else {
     serial_ptr = const_cast<atomic_uint_least32_t*>(&pi->serial);
   }