Reduce visibility of internal member function

This is the first change in a series that will eventually remove Sandboxed
API's use of multiple inheritance.

Drive-by:
- Rename short member names to full words
- Some reformatting
PiperOrigin-RevId: 402270954
Change-Id: I8af46b887921265a371b85603fd158ef3a8fab50

sandboxed_api/var_array.h

@@ -44,6 +44,7 @@ class Array : public Var, public Pointable {
         buffer_owned_(false) {
     SetLocal(const_cast<std::remove_const_t<T>*>(arr_));
   }
+
   // The array is allocated and owned by this object.
   explicit Array(size_t nelem)
       : nelem_(nelem), total_size_(nelem_ * sizeof(T)), buffer_owned_(true) {
@@ -52,6 +53,7 @@ class Array : public Var, public Pointable {
     SetLocal(storage);
     arr_ = static_cast<T*>(storage);
   }
+
   virtual ~Array() {
     if (buffer_owned_) {
       free(const_cast<std::remove_const_t<T>*>(arr_));
@@ -71,10 +73,6 @@ class Array : public Var, public Pointable {
                         " B., nelems: ", GetNElem());
   }
 
-  Ptr* CreatePtr(Pointable::SyncType type) override {
-    return new Ptr(this, type);
-  }
-
   // Resizes the local and remote buffer using realloc(). Note that this will
   // make all pointers to the current data (inside and outside of the sandbox)
   // invalid.
@@ -96,6 +94,12 @@ class Array : public Var, public Pointable {
   }
 
  private:
+  friend class LenVal;
+
+  Ptr* CreatePtr(Pointable::SyncType type) override {
+    return new Ptr(this, type);
+  }
+
   // Resizes the internal storage.
   absl::Status EnsureOwnedLocalBuffer(size_t size) {
     if (size % sizeof(T)) {
@@ -130,14 +134,9 @@ class Array : public Var, public Pointable {
 
   // Pointer to the data, owned by the object if buffer_owned_ is 'true'.
   T* arr_;
-  // Number of elements.
+  size_t nelem_;       // Number of elements
-  size_t nelem_;
+  size_t total_size_;  // Total size in bytes
-  // Total size in bytes.
+  bool buffer_owned_;  // Whether we own the buffer
-  size_t total_size_;
-  // Do we own the buffer?
-  bool buffer_owned_;
-
-  friend class LenVal;
 };
 
 // Specialized Array class for representing NUL-terminated C-style strings. The

sandboxed_api/var_int.h

@@ -32,7 +32,8 @@ class IntBase : public Reg<T>, public Pointable {
   IntBase() : IntBase(static_cast<T>(0)) {}
   explicit IntBase(T value) { this->SetValue(value); }
 
-  Ptr* CreatePtr(Pointable::SyncType type = Pointable::kSyncNone) override {
+ private:
+  Ptr* CreatePtr(Pointable::SyncType type) override {
     return new Ptr(this, type);
   }
 };

sandboxed_api/var_pointable.cc

@@ -17,6 +17,6 @@
 
 namespace sapi::v {
 
-void PtrDeleter::operator()(Ptr *p) { delete p; }
+void Pointable::PtrDeleter::operator()(Ptr *p) { delete p; }
 
 }  // namespace sapi::v

sandboxed_api/var_pointable.h

@@ -23,11 +23,6 @@ namespace sapi::v {
 
 class Ptr;
 
-// Needed so that we can use unique_ptr with incomplete type.
-struct PtrDeleter {
-  void operator()(Ptr* p);
-};
-
 // Class that implements pointer support for different objects.
 class Pointable {
  public:
@@ -46,9 +41,7 @@ class Pointable {
     kSyncBoth = kSyncBefore | kSyncAfter,
   };
 
-  // Necessary to implement creation of Ptr in inheriting class as it is
+  virtual ~Pointable() = default;
-  // incomplete type here.
-  virtual Ptr* CreatePtr(SyncType type) = 0;
 
   // Functions to get pointers with certain type of synchronization scheme.
   Ptr* PtrNone() {
@@ -80,9 +73,16 @@ class Pointable {
     return ptr_after_.get();
   }
 
-  virtual ~Pointable() = default;
-
  private:
+  // Needed so that we can use unique_ptr with incomplete type.
+  struct PtrDeleter {
+    void operator()(Ptr* p);
+  };
+
+  // Necessary to implement creation of Ptr in inheriting class as it is
+  // incomplete type here.
+  virtual Ptr* CreatePtr(SyncType sync_type) = 0;
+
   std::unique_ptr<Ptr, PtrDeleter> ptr_none_;
   std::unique_ptr<Ptr, PtrDeleter> ptr_both_;
   std::unique_ptr<Ptr, PtrDeleter> ptr_before_;

sandboxed_api/var_proto.h

@@ -53,10 +53,6 @@ class Proto : public Pointable, public Var {
   std::string GetTypeString() const final { return "Protobuf"; }
   std::string ToString() const final { return "Protobuf"; }
 
-  Ptr* CreatePtr(Pointable::SyncType type) override {
-    return new Ptr(this, type);
-  }
-
   void* GetRemote() const override { return wrapped_var_.GetRemote(); }
   void* GetLocal() const override { return wrapped_var_.GetLocal(); }
 
@@ -107,6 +103,10 @@ class Proto : public Pointable, public Var {
 
   explicit Proto(std::vector<uint8_t> data) : wrapped_var_(std::move(data)) {}
 
+  Ptr* CreatePtr(Pointable::SyncType type) override {
+    return new Ptr(this, type);
+  }
+
   // The management of reading/writing the data to the sandboxee is handled by
   // the LenVal class.
   LenVal wrapped_var_;

sandboxed_api/var_ptr.h

@@ -30,17 +30,16 @@ class Ptr : public Reg<Var*>, public Pointable {
  public:
   Ptr() = delete;
 
-  explicit Ptr(Var* val, SyncType sync_type)
+  explicit Ptr(Var* value, SyncType sync_type) : sync_type_(sync_type) {
-      : sync_type_(sync_type) {
+    Reg<Var*>::SetValue(value);
-    Reg<Var*>::SetValue(val);
   }
 
   Var* GetPointedVar() const { return Reg<Var*>::GetValue(); }
 
-  void SetValue(Var* ptr) final { val_->SetRemote(ptr); }
+  void SetValue(Var* ptr) final { value_->SetRemote(ptr); }
 
   Var* GetValue() const final {
-    return reinterpret_cast<Var*>(val_->GetRemote());
+    return reinterpret_cast<Var*>(value_->GetRemote());
   }
 
   const void* GetDataPtr() final {
@@ -66,11 +65,11 @@ class Ptr : public Reg<Var*>, public Pointable {
         var->GetRemote(), var->GetSize());
   }
 
-  Ptr* CreatePtr(Pointable::SyncType type = Pointable::kSyncNone) override {
+ private:
-    return new Ptr(this, type);
+  Ptr* CreatePtr(Pointable::SyncType sync_type) override {
+    return new Ptr(this, sync_type);
   }
 
- private:
   // GetDataPtr() interface requires of us to return a pointer to the data
   // (variable) that can be copied. We cannot get pointer to pointer with
   // Var::GetRemote(), hence we cache it, and return pointer to it.

sandboxed_api/var_reg.h

@@ -57,20 +57,20 @@ class Reg : public Callable {
                 "Only register-sized types are allowed as template argument "
                 "for class Reg.");
 
-  explicit Reg(const T val = {}) {
+  explicit Reg(const T value = {}) {
-    val_ = val;
+    value_ = value;
-    SetLocal(&val_);
+    SetLocal(&value_);
   }
 
   // Getter/Setter for the stored value.
-  virtual T GetValue() const { return val_; }
+  virtual T GetValue() const { return value_; }
-  virtual void SetValue(T val) { val_ = val; }
+  virtual void SetValue(T value) { value_ = value; }
 
   const void* GetDataPtr() override {
-    return reinterpret_cast<const void*>(&val_);
+    return reinterpret_cast<const void*>(&value_);
   }
   void SetDataFromPtr(const void* ptr, size_t max_sz) override {
-    memcpy(&val_, ptr, std::min(GetSize(), max_sz));
+    memcpy(&value_, ptr, std::min(GetSize(), max_sz));
   }
 
   size_t GetSize() const override { return sizeof(T); }
@@ -82,8 +82,7 @@ class Reg : public Callable {
   std::string ToString() const override;
 
  protected:
-  // The stored value.
+  T value_;  // The stored value.
-  T val_;
 };
 
 template <typename T>
@@ -117,13 +116,13 @@ std::string Reg<T>::GetTypeString() const {
 template <typename T>
 std::string Reg<T>::ToString() const {
   if constexpr (std::is_integral_v<T> || std::is_enum_v<T>) {
-    return absl::StrCat(val_);
+    return absl::StrCat(value_);
   }
   if constexpr (std::is_floating_point_v<T>) {
-    return absl::StrFormat("%.10f", val_);
+    return absl::StrFormat("%.10f", value_);
   }
   if constexpr (std::is_pointer<T>::value) {
-    return absl::StrFormat("%p", val_);
+    return absl::StrFormat("%p", value_);
   }
   // Not reached.
 }

sandboxed_api/var_struct.h

@@ -44,14 +44,15 @@ class Struct : public Var, public Pointable {
   const T& data() const { return struct_; }
   T* mutable_data() { return &struct_; }
 
+ protected:
+  friend class LenVal;
+
+  T struct_;
+
+ private:
   Ptr* CreatePtr(Pointable::SyncType type) override {
     return new Ptr(this, type);
   }
-
- protected:
-  T struct_;
-
-  friend class LenVal;
 };
 
 }  // namespace sapi::v

sandboxed_api/var_void.h

@@ -38,8 +38,9 @@ class Void : public Callable, public Pointable {
   const void* GetDataPtr() override { return nullptr; }
   void SetDataFromPtr(const void* ptr, size_t max_sz) override {}
 
-  Ptr* CreatePtr(Pointable::SyncType type = Pointable::kSyncNone) override {
+ private:
-    return new Ptr(this, type);
+  Ptr* CreatePtr(Pointable::SyncType sync_type) override {
+    return new Ptr(this, sync_type);
   }
 };