Move away from multiple inheritance

This change is a first step to make the SAPI variable hierarchy more sensible.
It turns the `Reg<T>` class into a descendant of `Pointable`, but without
making its `PtrXXX()` methods public (hence the `using` statements). Further
changes are needed to restructure this.

There are no functional changes and the class sizes, including vtables, should
not change.

PiperOrigin-RevId: 413333120
Change-Id: I90ceeaeb7aea482016f8f4bee81489d5a9db9ade

sandboxed_api/BUILD.bazel

@@ -131,7 +131,6 @@ cc_library(
         "var_abstract.cc",
         "var_int.cc",
         "var_lenval.cc",
-        "var_pointable.cc",
     ],
     hdrs = [
         "proto_helper.h",
@@ -140,7 +139,6 @@ cc_library(
         "var_array.h",
         "var_int.h",
         "var_lenval.h",
-        "var_pointable.h",
         "var_proto.h",
         "var_ptr.h",
         "var_reg.h",

sandboxed_api/CMakeLists.txt

@@ -139,8 +139,6 @@ add_library(sapi_vars ${SAPI_LIB_TYPE}
   var_int.h
   var_lenval.cc
   var_lenval.h
-  var_pointable.cc
-  var_pointable.h
   var_proto.h
   var_ptr.h
   var_reg.h

sandboxed_api/var_abstract.cc

@@ -18,11 +18,14 @@
 
 #include <sys/uio.h>
 
+#include <memory>
+
 #include <glog/logging.h>
 #include "absl/strings/str_cat.h"
 #include "sandboxed_api/rpcchannel.h"
 #include "sandboxed_api/sandbox2/comms.h"
 #include "sandboxed_api/util/status_macros.h"
+#include "sandboxed_api/var_ptr.h"
 
 namespace sapi::v {
 
@@ -32,6 +35,36 @@ Var::~Var() {
   }
 }
 
+void Var::PtrDeleter::operator()(Ptr* p) { delete p; }
+
+Ptr* Var::PtrNone() {
+  if (!ptr_none_) {
+    ptr_none_.reset(new Ptr(this, kSyncNone));
+  }
+  return ptr_none_.get();
+}
+
+Ptr* Var::PtrBoth() {
+  if (!ptr_both_) {
+    ptr_both_.reset(new Ptr(this, kSyncBoth));
+  }
+  return ptr_both_.get();
+}
+
+Ptr* Var::PtrBefore() {
+  if (!ptr_before_) {
+    ptr_before_.reset(new Ptr(this, kSyncBefore));
+  }
+  return ptr_before_.get();
+}
+
+Ptr* Var::PtrAfter() {
+  if (!ptr_after_) {
+    ptr_after_.reset(new Ptr(this, kSyncAfter));
+  }
+  return ptr_after_.get();
+}
+
 absl::Status Var::Allocate(RPCChannel* rpc_channel, bool automatic_free) {
   void* addr;
   SAPI_RETURN_IF_ERROR(rpc_channel->Allocate(GetSize(), &addr));

sandboxed_api/var_abstract.h

@@ -19,6 +19,7 @@
 #include <string>
 #include <type_traits>
 
+#include "absl/base/attributes.h"
 #include "absl/base/macros.h"
 #include "absl/status/status.h"
 #include "sandboxed_api/var_type.h"
@@ -36,8 +37,29 @@ namespace sapi::v {
 
 class Ptr;
 
+class ABSL_DEPRECATED(
+    "Use the Var::PtrXXX() family of methods instead") Pointable {
+ public:
+  enum SyncType {
+    // Do not synchronize the underlying object after/before calls.
+    kSyncNone = 0x0,
+    // Synchronize the underlying object (send the data to the sandboxee)
+    // before the call takes place.
+    kSyncBefore = 0x1,
+    // Synchronize the underlying object (retrieve data from the sandboxee)
+    // after the call has finished.
+    kSyncAfter = 0x2,
+    // Synchronize the underlying object with the remote object, by sending the
+    // data to the sandboxee before the call, and retrieving it from the
+    // sandboxee after the call has finished.
+    kSyncBoth = kSyncBefore | kSyncAfter,
+  };
+
+  virtual ~Pointable() = default;
+};
+
 // An abstract class representing variables.
-class Var {
+class Var : public Pointable {
  public:
   Var(const Var&) = delete;
   Var& operator=(const Var&) = delete;
@@ -68,6 +90,12 @@ class Var {
  protected:
   Var() = default;
 
+  // Functions to get pointers with certain type of synchronization schemes.
+  Ptr* PtrNone();
+  Ptr* PtrBoth();
+  Ptr* PtrBefore();
+  Ptr* PtrAfter();
+
   // Set pointer to local storage class.
   void SetLocal(void* local) { local_ = local; }
 
@@ -96,9 +124,19 @@ class Var {
                                              pid_t pid);
 
  private:
+  // Needed so that we can use unique_ptr with incomplete type.
+  struct PtrDeleter {
+    void operator()(Ptr* p);
+  };
+
   // Invokes Allocate()/Free()/Transfer*Sandboxee().
   friend class ::sapi::Sandbox;
 
+  std::unique_ptr<Ptr, PtrDeleter> ptr_none_;
+  std::unique_ptr<Ptr, PtrDeleter> ptr_both_;
+  std::unique_ptr<Ptr, PtrDeleter> ptr_before_;
+  std::unique_ptr<Ptr, PtrDeleter> ptr_after_;
+
   // Pointer to local storage of the variable.
   void* local_ = nullptr;
   // Pointer to remote storage of the variable.

sandboxed_api/var_array.h

@@ -27,15 +27,18 @@
 #include "sandboxed_api/rpcchannel.h"
 #include "sandboxed_api/util/status_macros.h"
 #include "sandboxed_api/var_abstract.h"
-#include "sandboxed_api/var_pointable.h"
 #include "sandboxed_api/var_ptr.h"
 
 namespace sapi::v {
 
 // Class representing an array.
 template <class T>
-class Array : public Var, public Pointable {
+class Array : public Var {
  public:
+  using Var::PtrAfter;
+  using Var::PtrBefore;
+  using Var::PtrBoth;
+
   // The array is not owned by this object.
   Array(T* arr, size_t nelem)
       : arr_(arr),
@@ -96,10 +99,6 @@ 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)) {

sandboxed_api/var_int.h

@@ -18,24 +18,21 @@
 #include <memory>
 
 #include "sandboxed_api/sandbox2/comms.h"
-#include "sandboxed_api/var_pointable.h"
 #include "sandboxed_api/var_ptr.h"
 #include "sandboxed_api/var_reg.h"
 
 namespace sapi::v {
 
-// Intermediate class for register sized variables
+// Intermediate class for register sized variables so we don't have to implement
-// so we don't have to implement ptr() everywhere.
+// ptr() everywhere.
 template <class T>
-class IntBase : public Reg<T>, public Pointable {
+class IntBase : public Reg<T> {
  public:
-  IntBase() : IntBase(static_cast<T>(0)) {}
+  using Var::PtrAfter;
-  explicit IntBase(T value) { this->SetValue(value); }
+  using Var::PtrBefore;
+  using Var::PtrBoth;
 
- private:
+  explicit IntBase(T value = {}) { this->SetValue(value); }
-  Ptr* CreatePtr(Pointable::SyncType type) override {
-    return new Ptr(this, type);
-  }
 };
 
 using Bool = IntBase<bool>;

sandboxed_api/var_lenval.h

@@ -23,7 +23,6 @@
 #include "sandboxed_api/lenval_core.h"
 #include "sandboxed_api/var_abstract.h"
 #include "sandboxed_api/var_array.h"
-#include "sandboxed_api/var_pointable.h"
 #include "sandboxed_api/var_ptr.h"
 #include "sandboxed_api/var_struct.h"
 
@@ -36,8 +35,12 @@ class Proto;
 // sandboxee which allows the bidirectional synchronization data structures with
 // changing lengths (e.g. protobuf structures). You probably want to directly
 // use protobufs as they are easier to handle.
-class LenVal : public Var, public Pointable {
+class LenVal : public Var {
  public:
+  using Var::PtrAfter;
+  using Var::PtrBefore;
+  using Var::PtrBoth;
+
   explicit LenVal(const char* data, uint64_t size)
       : array_(const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>(data)),
                size),
@@ -54,10 +57,6 @@ class LenVal : public Var, public Pointable {
   std::string GetTypeString() const final { return "LengthValue"; }
   std::string ToString() const final { return "LenVal"; }
 
-  Ptr* CreatePtr(Pointable::SyncType type) override {
-    return new Ptr(this, type);
-  }
-
   absl::Status ResizeData(RPCChannel* rpc_channel, size_t size);
   size_t GetDataSize() const { return struct_.data().size; }
   uint8_t* GetData() const { return array_.GetData(); }
@@ -65,6 +64,7 @@ class LenVal : public Var, public Pointable {
 
  protected:
   size_t GetSize() const final { return 0; }
+
   absl::Status Allocate(RPCChannel* rpc_channel, bool automatic_free) override;
   absl::Status Free(RPCChannel* rpc_channel) override;
   absl::Status TransferToSandboxee(RPCChannel* rpc_channel, pid_t pid) override;

sandboxed_api/var_pointable.cc

@@ -1,22 +0,0 @@
-// Copyright 2019 Google LLC
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "sandboxed_api/var_pointable.h"
-#include "sandboxed_api/var_ptr.h"
-
-namespace sapi::v {
-
-void Pointable::PtrDeleter::operator()(Ptr *p) { delete p; }
-
-}  // namespace sapi::v

sandboxed_api/var_pointable.h

@@ -1,94 +0,0 @@
-// Copyright 2019 Google LLC
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#ifndef SANDBOXED_API_VAR_POINTABLE_H_
-#define SANDBOXED_API_VAR_POINTABLE_H_
-
-#include <memory>
-
-#include "sandboxed_api/var_reg.h"
-
-namespace sapi::v {
-
-class Ptr;
-
-// Class that implements pointer support for different objects.
-class Pointable {
- public:
-  enum SyncType {
-    // Do not synchronize the underlying object after/before calls.
-    kSyncNone = 0x0,
-    // Synchronize the underlying object (send the data to the sandboxee)
-    // before the call takes place.
-    kSyncBefore = 0x1,
-    // Synchronize the underlying object (retrieve data from the sandboxee)
-    // after the call has finished.
-    kSyncAfter = 0x2,
-    // Synchronize the underlying object with the remote object, by sending the
-    // data to the sandboxee before the call, and retrieving it from the
-    // sandboxee after the call has finished.
-    kSyncBoth = kSyncBefore | kSyncAfter,
-  };
-
-  virtual ~Pointable() = default;
-
-  // Functions to get pointers with certain type of synchronization scheme.
-  Ptr* PtrNone() {
-    if (ptr_none_ == nullptr) {
-      ptr_none_.reset(CreatePtr(kSyncNone));
-    }
-
-    return ptr_none_.get();
-  }
-
-  Ptr* PtrBoth() {
-    if (ptr_both_ == nullptr) {
-      ptr_both_.reset(CreatePtr(kSyncBoth));
-    }
-    return ptr_both_.get();
-  }
-
-  Ptr* PtrBefore() {
-    if (ptr_before_ == nullptr) {
-      ptr_before_.reset(CreatePtr(kSyncBefore));
-    }
-    return ptr_before_.get();
-  }
-
-  Ptr* PtrAfter() {
-    if (ptr_after_ == nullptr) {
-      ptr_after_.reset(CreatePtr(kSyncAfter));
-    }
-    return ptr_after_.get();
-  }
-
- 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_;
-  std::unique_ptr<Ptr, PtrDeleter> ptr_after_;
-};
-
-}  // namespace sapi::v
-
-#endif  // SANDBOXED_API_VAR_POINTABLE_H_

sandboxed_api/var_proto.h

@@ -28,17 +28,20 @@
 #include "sandboxed_api/proto_helper.h"
 #include "sandboxed_api/util/status_macros.h"
 #include "sandboxed_api/var_lenval.h"
-#include "sandboxed_api/var_pointable.h"
 #include "sandboxed_api/var_ptr.h"
 
 namespace sapi::v {
 
 template <typename T>
-class Proto : public Pointable, public Var {
+class Proto : public Var {
  public:
   static_assert(std::is_base_of<google::protobuf::Message, T>::value,
                 "Template argument must be a proto message");
 
+  using Var::PtrAfter;
+  using Var::PtrBefore;
+  using Var::PtrBoth;
+
   ABSL_DEPRECATED("Use Proto<>::FromMessage() instead")
   explicit Proto(const T& proto)
       : wrapped_var_(SerializeProto(proto).value()) {}
@@ -103,10 +106,6 @@ 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

@@ -19,14 +19,14 @@
 
 #include "absl/base/macros.h"
 #include "absl/strings/str_format.h"
-#include "sandboxed_api/var_pointable.h"
+#include "sandboxed_api/var_abstract.h"
 #include "sandboxed_api/var_reg.h"
 
 namespace sapi::v {
 
 // Class representing a pointer. Takes both Var* and regular pointers in the
 // initializers.
-class Ptr : public Reg<Var*>, public Pointable {
+class Ptr : public Reg<Var*> {
  public:
   Ptr() = delete;
 
@@ -66,10 +66,6 @@ class Ptr : public Reg<Var*>, public Pointable {
   }
 
  private:
-  Ptr* CreatePtr(Pointable::SyncType sync_type) override {
-    return new Ptr(this, sync_type);
-  }
-
   // 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

@@ -29,9 +29,6 @@ namespace sapi::v {
 // type specifier in methods.
 class Callable : public Var {
  public:
-  Callable(const Callable&) = delete;
-  Callable& operator=(const Callable&) = delete;
-
   // Get pointer to the stored data.
   virtual const void* GetDataPtr() = 0;
 
@@ -82,7 +79,8 @@ class Reg : public Callable {
   std::string ToString() const override;
 
  protected:
-  T value_;  // The stored value.
+  // The stored value.
+  T value_;
 };
 
 template <typename T>

sandboxed_api/var_struct.h

@@ -19,15 +19,18 @@
 
 #include "absl/base/macros.h"
 #include "absl/strings/str_cat.h"
-#include "sandboxed_api/var_pointable.h"
 #include "sandboxed_api/var_ptr.h"
 
 namespace sapi::v {
 
 // Class representing a structure.
 template <class T>
-class Struct : public Var, public Pointable {
+class Struct : public Var {
  public:
+  using Var::PtrAfter;
+  using Var::PtrBefore;
+  using Var::PtrBoth;
+
   // Forwarding constructor to initalize the struct_ field.
   template <typename... Args>
   explicit Struct(Args&&... args) : struct_(std::forward<Args>(args)...) {
@@ -48,11 +51,6 @@ class Struct : public Var, public Pointable {
   friend class LenVal;
 
   T struct_;
-
- private:
-  Ptr* CreatePtr(Pointable::SyncType type) override {
-    return new Ptr(this, type);
-  }
 };
 
 }  // namespace sapi::v

sandboxed_api/var_void.h

@@ -23,13 +23,10 @@
 namespace sapi::v {
 
 // Good, old void.
-class Void : public Callable, public Pointable {
+class Void : public Callable {
  public:
   Void() = default;
 
-  Void(const Void&) = delete;
-  Void& operator=(const Void&) = delete;
-
   size_t GetSize() const final { return 0U; }
   Type GetType() const final { return Type::kVoid; }
   std::string GetTypeString() const final { return "Void"; }
@@ -37,11 +34,6 @@ class Void : public Callable, public Pointable {
 
   const void* GetDataPtr() override { return nullptr; }
   void SetDataFromPtr(const void* ptr, size_t max_sz) override {}
-
- private:
-  Ptr* CreatePtr(Pointable::SyncType sync_type) override {
-    return new Ptr(this, sync_type);
-  }
 };
 
 }  // namespace sapi::v

sandboxed_api/vars.h

@@ -18,7 +18,6 @@
 #include "sandboxed_api/var_array.h"
 #include "sandboxed_api/var_int.h"
 #include "sandboxed_api/var_lenval.h"
-#include "sandboxed_api/var_pointable.h"
 #include "sandboxed_api/var_proto.h"
 #include "sandboxed_api/var_ptr.h"
 #include "sandboxed_api/var_struct.h"