CodeGen: refactor API [1/3]

sandbox/linux/seccomp-bpf/codegen.cc

Index: sandbox/linux/seccomp-bpf/codegen.cc
diff --git a/sandbox/linux/seccomp-bpf/codegen.cc b/sandbox/linux/seccomp-bpf/codegen.cc
index 1ee79b6ed0e9031e4b4104ba2ab8bbbea4fd433a..6bf71fb6142e8ec0cedddc8e9d810909b91e096e 100644
--- a/sandbox/linux/seccomp-bpf/codegen.cc
+++ b/sandbox/linux/seccomp-bpf/codegen.cc
@@ -15,6 +15,11 @@
 
 namespace sandbox {
 
+// Unfortunately this needs to be defined out-of-line because inline
+// initializing a static member to "nullptr" requires "constexpr",
+// which is currently banned by the Chromium style guide.
+const CodeGen::Addr CodeGen::kNullAddr = nullptr;
+
 CodeGen::CodeGen() : compiled_(false) {}
 
 CodeGen::~CodeGen() {
@@ -30,47 +35,28 @@ CodeGen::~CodeGen() {
   }
 }
 
-Instruction* CodeGen::MakeInstruction(uint16_t code,
-                                      uint32_t k,
-                                      Instruction* next) {
-  // We can handle non-jumping instructions and "always" jumps. Both of
-  // them are followed by exactly one "next" instruction.
-  // We allow callers to defer specifying "next", but then they must call
-  // "joinInstructions" later.
-  if (BPF_CLASS(code) == BPF_JMP && BPF_OP(code) != BPF_JA) {
-    SANDBOX_DIE(
-        "Must provide both \"true\" and \"false\" branch "
-        "for a BPF_JMP");
-  }
-  if (next && BPF_CLASS(code) == BPF_RET) {
-    SANDBOX_DIE("Cannot append instructions after a return statement");
-  }
+CodeGen::Addr CodeGen::MakeInstruction(uint16_t code,
+                                       uint32_t k,
+                                       Addr jt,
+                                       Addr jf) {
+  Instruction* insn;

[jln (very slow on Chromium), 2014/11/11 08:40:48]

Addr

[mdempsky, 2014/11/17 22:11:06]

Done.

   if (BPF_CLASS(code) == BPF_JMP) {
-    // "Always" jumps use the "true" branch target, only.
-    Instruction* insn = new Instruction(code, 0, next, NULL);
-    instructions_.push_back(insn);
-    return insn;
+    CHECK_NE(kNullAddr, jt);
+    if (BPF_OP(code) == BPF_JA) {
+      CHECK_EQ(kNullAddr, jf);
+    } else {
+      CHECK_NE(kNullAddr, jf);
+    }
+    insn = new Instruction(code, k, jt, jf);
   } else {
-    // Non-jumping instructions do not use any of the branch targets.
-    Instruction* insn = new Instruction(code, k, next);
-    instructions_.push_back(insn);
-    return insn;
-  }
-}
-
-Instruction* CodeGen::MakeInstruction(uint16_t code,
-                                      uint32_t k,
-                                      Instruction* jt,
-                                      Instruction* jf) {
-  // We can handle all conditional jumps. They are followed by both a
-  // "true" and a "false" branch.
-  if (BPF_CLASS(code) != BPF_JMP || BPF_OP(code) == BPF_JA) {
-    SANDBOX_DIE("Expected a BPF_JMP instruction");
-  }
-  if (!jt || !jf) {
-    SANDBOX_DIE("Branches must jump to a valid instruction");
+    if (BPF_CLASS(code) == BPF_RET) {
+      CHECK_EQ(kNullAddr, jt);
+    } else {
+      CHECK_NE(kNullAddr, jt);
+    }
+    CHECK_EQ(kNullAddr, jf);
+    insn = new Instruction(code, k, jt);
   }
-  Instruction* insn = new Instruction(code, k, jt, jf);
   instructions_.push_back(insn);
   return insn;
 }