sandbox: style cleanup
Based on readability review by Dean Berris at Google.
R=jln@chromium.org
Review URL: https://codereview.chromium.org/511993005
diff --git a/sandbox/linux/bpf_dsl/bpf_dsl.cc b/sandbox/linux/bpf_dsl/bpf_dsl.cc
index e9057aed..ac16051c 100644
--- a/sandbox/linux/bpf_dsl/bpf_dsl.cc
+++ b/sandbox/linux/bpf_dsl/bpf_dsl.cc
@@ -6,67 +6,75 @@
#include <errno.h>
+#include <limits>
+
#include "base/logging.h"
#include "base/memory/ref_counted.h"
#include "sandbox/linux/seccomp-bpf/errorcode.h"
#include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
-using namespace sandbox::bpf_dsl::internal;
-typedef ::sandbox::Trap::TrapFnc TrapFnc;
-
namespace sandbox {
namespace bpf_dsl {
namespace {
-class AllowResultExprImpl : public ResultExprImpl {
+class AllowResultExprImpl : public internal::ResultExprImpl {
public:
AllowResultExprImpl() {}
+
virtual ErrorCode Compile(SandboxBPF* sb) const OVERRIDE {
return ErrorCode(ErrorCode::ERR_ALLOWED);
}
private:
virtual ~AllowResultExprImpl() {}
+
DISALLOW_COPY_AND_ASSIGN(AllowResultExprImpl);
};
-class ErrorResultExprImpl : public ResultExprImpl {
+class ErrorResultExprImpl : public internal::ResultExprImpl {
public:
explicit ErrorResultExprImpl(int err) : err_(err) {
CHECK(err_ >= ErrorCode::ERR_MIN_ERRNO && err_ <= ErrorCode::ERR_MAX_ERRNO);
}
+
virtual ErrorCode Compile(SandboxBPF* sb) const OVERRIDE {
return ErrorCode(err_);
}
private:
virtual ~ErrorResultExprImpl() {}
+
int err_;
+
DISALLOW_COPY_AND_ASSIGN(ErrorResultExprImpl);
};
-class TrapResultExprImpl : public ResultExprImpl {
+class TrapResultExprImpl : public internal::ResultExprImpl {
public:
- TrapResultExprImpl(TrapFnc func, void* arg) : func_(func), arg_(arg) {
+ TrapResultExprImpl(Trap::TrapFnc func, void* arg) : func_(func), arg_(arg) {
DCHECK(func_);
}
+
virtual ErrorCode Compile(SandboxBPF* sb) const OVERRIDE {
return sb->Trap(func_, arg_);
}
private:
virtual ~TrapResultExprImpl() {}
- TrapFnc func_;
+
+ Trap::TrapFnc func_;
void* arg_;
+
DISALLOW_COPY_AND_ASSIGN(TrapResultExprImpl);
};
-class IfThenResultExprImpl : public ResultExprImpl {
+class IfThenResultExprImpl : public internal::ResultExprImpl {
public:
- IfThenResultExprImpl(BoolExpr cond,
- ResultExpr then_result,
- ResultExpr else_result)
+ IfThenResultExprImpl(const BoolExpr& cond,
+ const ResultExpr& then_result,
+ const ResultExpr& else_result)
: cond_(cond), then_result_(then_result), else_result_(else_result) {}
+
virtual ErrorCode Compile(SandboxBPF* sb) const OVERRIDE {
return cond_->Compile(
sb, then_result_->Compile(sb), else_result_->Compile(sb));
@@ -74,19 +82,22 @@
private:
virtual ~IfThenResultExprImpl() {}
+
BoolExpr cond_;
ResultExpr then_result_;
ResultExpr else_result_;
+
DISALLOW_COPY_AND_ASSIGN(IfThenResultExprImpl);
};
-class PrimitiveBoolExprImpl : public BoolExprImpl {
+class PrimitiveBoolExprImpl : public internal::BoolExprImpl {
public:
PrimitiveBoolExprImpl(int argno,
ErrorCode::ArgType is_32bit,
ErrorCode::Operation op,
uint64_t value)
: argno_(argno), is_32bit_(is_32bit), op_(op), value_(value) {}
+
virtual ErrorCode Compile(SandboxBPF* sb,
ErrorCode true_ec,
ErrorCode false_ec) const OVERRIDE {
@@ -95,16 +106,19 @@
private:
virtual ~PrimitiveBoolExprImpl() {}
+
int argno_;
ErrorCode::ArgType is_32bit_;
ErrorCode::Operation op_;
uint64_t value_;
+
DISALLOW_COPY_AND_ASSIGN(PrimitiveBoolExprImpl);
};
-class NegateBoolExprImpl : public BoolExprImpl {
+class NegateBoolExprImpl : public internal::BoolExprImpl {
public:
- explicit NegateBoolExprImpl(BoolExpr cond) : cond_(cond) {}
+ explicit NegateBoolExprImpl(const BoolExpr& cond) : cond_(cond) {}
+
virtual ErrorCode Compile(SandboxBPF* sb,
ErrorCode true_ec,
ErrorCode false_ec) const OVERRIDE {
@@ -113,13 +127,17 @@
private:
virtual ~NegateBoolExprImpl() {}
+
BoolExpr cond_;
+
DISALLOW_COPY_AND_ASSIGN(NegateBoolExprImpl);
};
-class AndBoolExprImpl : public BoolExprImpl {
+class AndBoolExprImpl : public internal::BoolExprImpl {
public:
- AndBoolExprImpl(BoolExpr lhs, BoolExpr rhs) : lhs_(lhs), rhs_(rhs) {}
+ AndBoolExprImpl(const BoolExpr& lhs, const BoolExpr& rhs)
+ : lhs_(lhs), rhs_(rhs) {}
+
virtual ErrorCode Compile(SandboxBPF* sb,
ErrorCode true_ec,
ErrorCode false_ec) const OVERRIDE {
@@ -128,13 +146,18 @@
private:
virtual ~AndBoolExprImpl() {}
- BoolExpr lhs_, rhs_;
+
+ BoolExpr lhs_;
+ BoolExpr rhs_;
+
DISALLOW_COPY_AND_ASSIGN(AndBoolExprImpl);
};
-class OrBoolExprImpl : public BoolExprImpl {
+class OrBoolExprImpl : public internal::BoolExprImpl {
public:
- OrBoolExprImpl(BoolExpr lhs, BoolExpr rhs) : lhs_(lhs), rhs_(rhs) {}
+ OrBoolExprImpl(const BoolExpr& lhs, const BoolExpr& rhs)
+ : lhs_(lhs), rhs_(rhs) {}
+
virtual ErrorCode Compile(SandboxBPF* sb,
ErrorCode true_ec,
ErrorCode false_ec) const OVERRIDE {
@@ -143,7 +166,10 @@
private:
virtual ~OrBoolExprImpl() {}
- BoolExpr lhs_, rhs_;
+
+ BoolExpr lhs_;
+ BoolExpr rhs_;
+
DISALLOW_COPY_AND_ASSIGN(OrBoolExprImpl);
};
@@ -161,23 +187,27 @@
const ErrorCode::ArgType arg_type =
(size <= 4) ? ErrorCode::TP_32BIT : ErrorCode::TP_64BIT;
- if (mask == static_cast<uint64_t>(-1)) {
+ if (mask == std::numeric_limits<uint64_t>::max()) {
// Arg == Val
return BoolExpr(new const PrimitiveBoolExprImpl(
num, arg_type, ErrorCode::OP_EQUAL, val));
- } else if (mask == val) {
+ }
+
+ if (mask == val) {
// (Arg & Mask) == Mask
return BoolExpr(new const PrimitiveBoolExprImpl(
num, arg_type, ErrorCode::OP_HAS_ALL_BITS, mask));
- } else if (val == 0) {
+ }
+
+ if (val == 0) {
// (Arg & Mask) == 0, which is semantically equivalent to !((arg & mask) !=
// 0).
return !BoolExpr(new const PrimitiveBoolExprImpl(
num, arg_type, ErrorCode::OP_HAS_ANY_BITS, mask));
- } else {
- CHECK(false) << "Unimplemented ArgEq case";
- return BoolExpr();
}
+
+ CHECK(false) << "Unimplemented ArgEq case";
+ return BoolExpr();
}
} // namespace internal
@@ -190,23 +220,23 @@
return ResultExpr(new const ErrorResultExprImpl(err));
}
-ResultExpr Trap(TrapFnc trap_func, void* aux) {
+ResultExpr Trap(Trap::TrapFnc trap_func, void* aux) {
return ResultExpr(new const TrapResultExprImpl(trap_func, aux));
}
-BoolExpr operator!(BoolExpr cond) {
+BoolExpr operator!(const BoolExpr& cond) {
return BoolExpr(new const NegateBoolExprImpl(cond));
}
-BoolExpr operator&&(BoolExpr lhs, BoolExpr rhs) {
+BoolExpr operator&&(const BoolExpr& lhs, const BoolExpr& rhs) {
return BoolExpr(new const AndBoolExprImpl(lhs, rhs));
}
-BoolExpr operator||(BoolExpr lhs, BoolExpr rhs) {
+BoolExpr operator||(const BoolExpr& lhs, const BoolExpr& rhs) {
return BoolExpr(new const OrBoolExprImpl(lhs, rhs));
}
-Elser If(BoolExpr cond, ResultExpr then_result) {
+Elser If(const BoolExpr& cond, const ResultExpr& then_result) {
return Elser(Cons<Elser::Clause>::List()).ElseIf(cond, then_result);
}
@@ -219,12 +249,12 @@
Elser::~Elser() {
}
-Elser Elser::ElseIf(BoolExpr cond, ResultExpr then_result) const {
+Elser Elser::ElseIf(const BoolExpr& cond, const ResultExpr& then_result) const {
return Elser(
Cons<Clause>::Make(std::make_pair(cond, then_result), clause_list_));
}
-ResultExpr Elser::Else(ResultExpr else_result) const {
+ResultExpr Elser::Else(const ResultExpr& else_result) const {
// We finally have the default result expression for this
// if/then/else sequence. Also, we've already accumulated all
// if/then pairs into a list of reverse order (i.e., lower priority
@@ -269,8 +299,7 @@
return InvalidSyscall()->Compile(sb);
}
-ResultExpr SandboxBPFDSLPolicy::Trap(::sandbox::Trap::TrapFnc trap_func,
- void* aux) {
+ResultExpr SandboxBPFDSLPolicy::Trap(Trap::TrapFnc trap_func, void* aux) {
return bpf_dsl::Trap(trap_func, aux);
}
diff --git a/sandbox/linux/bpf_dsl/bpf_dsl.h b/sandbox/linux/bpf_dsl/bpf_dsl.h
index d46102a..da90efd 100644
--- a/sandbox/linux/bpf_dsl/bpf_dsl.h
+++ b/sandbox/linux/bpf_dsl/bpf_dsl.h
@@ -7,6 +7,7 @@
#include <stdint.h>
+#include <limits>
#include <utility>
#include "base/macros.h"
@@ -108,7 +109,7 @@
virtual ErrorCode InvalidSyscall(SandboxBPF* sb) const OVERRIDE FINAL;
// Helper method so policies can just write Trap(func, aux).
- static ResultExpr Trap(::sandbox::Trap::TrapFnc trap_func, void* aux);
+ static ResultExpr Trap(Trap::TrapFnc trap_func, void* aux);
private:
DISALLOW_COPY_AND_ASSIGN(SandboxBPFDSLPolicy);
@@ -127,41 +128,48 @@
// Trap specifies a result that the system call should be handled by
// trapping back into userspace and invoking |trap_func|, passing
// |aux| as the second parameter.
-SANDBOX_EXPORT ResultExpr Trap(::sandbox::Trap::TrapFnc trap_func, void* aux);
+SANDBOX_EXPORT ResultExpr Trap(Trap::TrapFnc trap_func, void* aux);
template <typename T>
class SANDBOX_EXPORT Arg {
public:
// Initializes the Arg to represent the |num|th system call
// argument (indexed from 0), which is of type |T|.
- explicit Arg(int num) : num_(num), mask_(-1) {}
+ explicit Arg(int num)
+ : num_(num), mask_(std::numeric_limits<uint64_t>::max()) {}
Arg(const Arg& arg) : num_(arg.num_), mask_(arg.mask_) {}
// Returns an Arg representing the current argument, but after
// bitwise-and'ing it with |rhs|.
- Arg operator&(uint64_t rhs) const { return Arg(num_, mask_ & rhs); }
+ friend Arg operator&(const Arg& lhs, uint64_t rhs) {
+ return Arg(lhs.num_, lhs.mask_ & rhs);
+ }
// Returns a boolean expression comparing whether the system call
// argument (after applying any bitmasks, if appropriate) equals |rhs|.
- BoolExpr operator==(T rhs) const;
+ friend BoolExpr operator==(const Arg& lhs, T rhs) { return lhs.EqualTo(rhs); }
private:
Arg(int num, uint64_t mask) : num_(num), mask_(mask) {}
+
+ BoolExpr EqualTo(T val) const;
+
int num_;
uint64_t mask_;
+
DISALLOW_ASSIGN(Arg);
};
// Various ways to combine boolean expressions into more complex expressions.
// They follow standard boolean algebra laws.
-SANDBOX_EXPORT BoolExpr operator!(BoolExpr cond);
-SANDBOX_EXPORT BoolExpr operator&&(BoolExpr lhs, BoolExpr rhs);
-SANDBOX_EXPORT BoolExpr operator||(BoolExpr lhs, BoolExpr rhs);
+SANDBOX_EXPORT BoolExpr operator!(const BoolExpr& cond);
+SANDBOX_EXPORT BoolExpr operator&&(const BoolExpr& lhs, const BoolExpr& rhs);
+SANDBOX_EXPORT BoolExpr operator||(const BoolExpr& lhs, const BoolExpr& rhs);
// If begins a conditional result expression predicated on the
// specified boolean expression.
-SANDBOX_EXPORT Elser If(BoolExpr cond, ResultExpr then_result);
+SANDBOX_EXPORT Elser If(const BoolExpr& cond, const ResultExpr& then_result);
class SANDBOX_EXPORT Elser {
public:
@@ -170,17 +178,20 @@
// ElseIf extends the conditional result expression with another
// "if then" clause, predicated on the specified boolean expression.
- Elser ElseIf(BoolExpr cond, ResultExpr then_result) const;
+ Elser ElseIf(const BoolExpr& cond, const ResultExpr& then_result) const;
// Else terminates a conditional result expression using |else_result| as
// the default fallback result expression.
- ResultExpr Else(ResultExpr else_result) const;
+ ResultExpr Else(const ResultExpr& else_result) const;
private:
typedef std::pair<BoolExpr, ResultExpr> Clause;
+
explicit Elser(Cons<Clause>::List clause_list);
+
Cons<Clause>::List clause_list_;
- friend Elser If(BoolExpr, ResultExpr);
+
+ friend Elser If(const BoolExpr&, const ResultExpr&);
DISALLOW_ASSIGN(Elser);
};
@@ -235,9 +246,13 @@
// Definition requires ArgEq to have been declared. Moved out-of-line
// to minimize how much internal clutter users have to ignore while
// reading the header documentation.
+//
+// Additionally, we use this helper member function to avoid linker errors
+// caused by defining operator== out-of-line. For a more detailed explanation,
+// see http://www.parashift.com/c++-faq-lite/template-friends.html.
template <typename T>
-BoolExpr Arg<T>::operator==(T rhs) const {
- return internal::ArgEq(num_, sizeof(T), mask_, static_cast<uint64_t>(rhs));
+BoolExpr Arg<T>::EqualTo(T val) const {
+ return internal::ArgEq(num_, sizeof(T), mask_, static_cast<uint64_t>(val));
}
} // namespace bpf_dsl
diff --git a/sandbox/linux/bpf_dsl/bpf_dsl_unittest.cc b/sandbox/linux/bpf_dsl/bpf_dsl_unittest.cc
index 560d94c..d975d64 100644
--- a/sandbox/linux/bpf_dsl/bpf_dsl_unittest.cc
+++ b/sandbox/linux/bpf_dsl/bpf_dsl_unittest.cc
@@ -16,8 +16,6 @@
#include "sandbox/linux/seccomp-bpf/sandbox_bpf_policy.h"
#include "sandbox/linux/seccomp-bpf/syscall.h"
-using namespace sandbox::bpf_dsl;
-
// Helper macro to assert that invoking system call |sys| directly via
// Syscall::Call with arguments |...| returns |res|.
// Errors can be asserted by specifying a value like "-EINVAL".
@@ -25,6 +23,7 @@
BPF_ASSERT_EQ(res, Stubs::sys(__VA_ARGS__))
namespace sandbox {
+namespace bpf_dsl {
namespace {
// Type safe stubs for tested system calls.
@@ -265,4 +264,5 @@
}
} // namespace
+} // namespace bpf_dsl
} // namespace sandbox
