Modernize a few files

- Use default initialization
- Rely on `static_assert()` and use `if constexpr` when checking SAPI
  variable type
- Small style fixes

PiperOrigin-RevId: 322107281
Change-Id: I48cf43f354b60e31e6207552dbbfa16e3acd5615
This commit is contained in:
Christian Blichmann 2020-07-20 03:07:15 -07:00 committed by Copybara-Service
parent eb62bae167
commit c7a27dd4b1
5 changed files with 28 additions and 30 deletions

View File

@ -147,7 +147,7 @@ TEST(SAPITest, HasStackTraces) {
// Various tests: // Various tests:
// Leaks a file descriptor inside the sandboxee. // Leaks a file descriptor inside the sandboxee.
int leak_file_descriptor(sapi::Sandbox* sandbox, const char* path) { int LeakFileDescriptor(sapi::Sandbox* sandbox, const char* path) {
int raw_fd = open(path, O_RDONLY); int raw_fd = open(path, O_RDONLY);
sapi::v::Fd fd(raw_fd); // Takes ownership of the raw fd. sapi::v::Fd fd(raw_fd); // Takes ownership of the raw fd.
EXPECT_THAT(sandbox->TransferToSandboxee(&fd), IsOk()); EXPECT_THAT(sandbox->TransferToSandboxee(&fd), IsOk());
@ -162,12 +162,12 @@ TEST(SandboxTest, RestartSandboxFD) {
auto test_body = [](sapi::Sandbox* sandbox) -> absl::Status { auto test_body = [](sapi::Sandbox* sandbox) -> absl::Status {
// Open some FDs and check their value. // Open some FDs and check their value.
EXPECT_THAT(leak_file_descriptor(sandbox, "/proc/self/exe"), Eq(3)); EXPECT_THAT(LeakFileDescriptor(sandbox, "/proc/self/exe"), Eq(3));
EXPECT_THAT(leak_file_descriptor(sandbox, "/proc/self/exe"), Eq(4)); EXPECT_THAT(LeakFileDescriptor(sandbox, "/proc/self/exe"), Eq(4));
SAPI_RETURN_IF_ERROR(sandbox->Restart(false)); SAPI_RETURN_IF_ERROR(sandbox->Restart(false));
// We should have a fresh sandbox now = FDs open previously should be // We should have a fresh sandbox now = FDs open previously should be
// closed now. // closed now.
EXPECT_THAT(leak_file_descriptor(sandbox, "/proc/self/exe"), Eq(3)); EXPECT_THAT(LeakFileDescriptor(sandbox, "/proc/self/exe"), Eq(3));
return absl::OkStatus(); return absl::OkStatus();
}; };
@ -178,13 +178,13 @@ TEST(SandboxTest, RestartTransactionSandboxFD) {
sapi::BasicTransaction st{absl::make_unique<SumSandbox>()}; sapi::BasicTransaction st{absl::make_unique<SumSandbox>()};
EXPECT_THAT(st.Run([](sapi::Sandbox* sandbox) -> absl::Status { EXPECT_THAT(st.Run([](sapi::Sandbox* sandbox) -> absl::Status {
EXPECT_THAT(leak_file_descriptor(sandbox, "/proc/self/exe"), Eq(3)); EXPECT_THAT(LeakFileDescriptor(sandbox, "/proc/self/exe"), Eq(3));
return absl::OkStatus(); return absl::OkStatus();
}), }),
IsOk()); IsOk());
EXPECT_THAT(st.Run([](sapi::Sandbox* sandbox) -> absl::Status { EXPECT_THAT(st.Run([](sapi::Sandbox* sandbox) -> absl::Status {
EXPECT_THAT(leak_file_descriptor(sandbox, "/proc/self/exe"), Eq(4)); EXPECT_THAT(LeakFileDescriptor(sandbox, "/proc/self/exe"), Eq(4));
return absl::OkStatus(); return absl::OkStatus();
}), }),
IsOk()); IsOk());
@ -192,7 +192,7 @@ TEST(SandboxTest, RestartTransactionSandboxFD) {
EXPECT_THAT(st.Restart(), IsOk()); EXPECT_THAT(st.Restart(), IsOk());
EXPECT_THAT(st.Run([](sapi::Sandbox* sandbox) -> absl::Status { EXPECT_THAT(st.Run([](sapi::Sandbox* sandbox) -> absl::Status {
EXPECT_THAT(leak_file_descriptor(sandbox, "/proc/self/exe"), Eq(3)); EXPECT_THAT(LeakFileDescriptor(sandbox, "/proc/self/exe"), Eq(3));
return absl::OkStatus(); return absl::OkStatus();
}), }),
IsOk()); IsOk());

View File

@ -66,7 +66,7 @@ class Var {
virtual ~Var(); virtual ~Var();
protected: protected:
Var() : local_(nullptr), remote_(nullptr), free_rpc_channel_(nullptr) {} Var() = default;
// Set pointer to local storage class. // Set pointer to local storage class.
void SetLocal(void* local) { local_ = local; } void SetLocal(void* local) { local_ = local; }
@ -96,17 +96,17 @@ class Var {
pid_t pid); pid_t pid);
private: private:
// Invokes Allocate()/Free()/Transfer*Sandboxee().
friend class ::sapi::Sandbox;
// Pointer to local storage of the variable. // Pointer to local storage of the variable.
void* local_; void* local_ = nullptr;
// Pointer to remote storage of the variable. // Pointer to remote storage of the variable.
void* remote_; void* remote_ = nullptr;
// Comms which can be used to free resources allocated in the sandboxer upon // Comms which can be used to free resources allocated in the sandboxer upon
// this process' end of lifetime. // this process' end of lifetime.
RPCChannel* free_rpc_channel_; RPCChannel* free_rpc_channel_ = nullptr;
// Invokes Allocate()/Free()/Transfer*Sandboxee().
friend class ::sapi::Sandbox;
}; };
} // namespace sapi::v } // namespace sapi::v

View File

@ -67,9 +67,7 @@ class GenericPtr : public IntBase<uintptr_t> {
class Fd : public Int { class Fd : public Int {
public: public:
Type GetType() const override { return Type::kFd; } Type GetType() const override { return Type::kFd; }
explicit Fd(int val) : remote_fd_(-1), own_local_(true), own_remote_(true) { explicit Fd(int val) { SetValue(val); }
SetValue(val);
}
~Fd() override; ~Fd() override;
// Getter and setter of remote file descriptor. // Getter and setter of remote file descriptor.
@ -95,9 +93,9 @@ class Fd : public Int {
absl::Status TransferToSandboxee(RPCChannel* rpc_channel, pid_t pid) override; absl::Status TransferToSandboxee(RPCChannel* rpc_channel, pid_t pid) override;
private: private:
int remote_fd_; int remote_fd_ = -1;
bool own_local_; bool own_local_ = true;
bool own_remote_; bool own_remote_ = true;
}; };
} // namespace sapi::v } // namespace sapi::v

View File

@ -47,12 +47,12 @@ class Callable : public Var {
// class Reg represents register-sized variables. // class Reg represents register-sized variables.
template <typename T> template <typename T>
class Reg : public Callable { class Reg : public Callable {
public:
static_assert(std::is_integral<T>() || std::is_floating_point<T>() || static_assert(std::is_integral<T>() || std::is_floating_point<T>() ||
std::is_pointer<T>() || std::is_enum<T>(), std::is_pointer<T>() || std::is_enum<T>(),
"Only register-sized types are allowed as template argument " "Only register-sized types are allowed as template argument "
"for class Reg."); "for class Reg.");
public:
Reg() : Reg(static_cast<T>(0)) {} Reg() : Reg(static_cast<T>(0)) {}
explicit Reg(const T val) { explicit Reg(const T val) {
@ -74,29 +74,29 @@ class Reg : public Callable {
size_t GetSize() const override { return sizeof(T); } size_t GetSize() const override { return sizeof(T); }
Type GetType() const override { Type GetType() const override {
if (std::is_integral<T>() || std::is_enum<T>()) { if constexpr (std::is_integral<T>() || std::is_enum<T>()) {
return Type::kInt; return Type::kInt;
} }
if (std::is_floating_point<T>()) { if constexpr (std::is_floating_point<T>()) {
return Type::kFloat; return Type::kFloat;
} }
if (std::is_pointer<T>()) { if constexpr (std::is_pointer<T>()) {
return Type::kPointer; return Type::kPointer;
} }
LOG(FATAL) << "Incorrect type"; // Not reached
} }
std::string GetTypeString() const override { std::string GetTypeString() const override {
if (std::is_integral<T>() || std::is_enum<T>()) { if constexpr (std::is_integral<T>() || std::is_enum<T>()) {
return "Integer"; return "Integer";
} }
if (std::is_floating_point<T>()) { if constexpr (std::is_floating_point<T>()) {
return "Floating-point"; return "Floating-point";
} }
if (std::is_pointer<T>()) { if constexpr (std::is_pointer<T>()) {
return "Pointer"; return "Pointer";
} }
LOG(FATAL) << "Incorrect type"; // Not reached
} }
std::string ToString() const override { return ValToString(); } std::string ToString() const override { return ValToString(); }

View File

@ -34,7 +34,7 @@ class Struct : public Var, public Pointable {
SetLocal(&struct_); SetLocal(&struct_);
} }
size_t GetSize() const final { return sizeof(struct_); } size_t GetSize() const final { return sizeof(T); }
Type GetType() const final { return Type::kStruct; } Type GetType() const final { return Type::kStruct; }
std::string GetTypeString() const final { return "Structure"; } std::string GetTypeString() const final { return "Structure"; }
std::string ToString() const final { std::string ToString() const final {