CodeGen: refactor API [1/3]

sandbox/linux/seccomp-bpf/codegen.h

Index: sandbox/linux/seccomp-bpf/codegen.h
diff --git a/sandbox/linux/seccomp-bpf/codegen.h b/sandbox/linux/seccomp-bpf/codegen.h
index 671b09e4264d3db8ba5a5fdec61d58b0d7b25aca..7061654b65f9acc772eb9c41137e59cd92d12116 100644
--- a/sandbox/linux/seccomp-bpf/codegen.h
+++ b/sandbox/linux/seccomp-bpf/codegen.h
@@ -34,7 +34,7 @@ typedef std::map<const BasicBlock*, int> IncomingBranches;
 // this DAG to a Program.
 //
 //   CodeGen gen;
-//   Instruction *allow, *branch, *dag;
+//   CodeGen::Addr allow, branch, dag;
 //
 //   allow =
 //     gen.MakeInstruction(BPF_RET+BPF_K,
@@ -60,6 +60,14 @@ class SANDBOX_EXPORT CodeGen {
   // program in the kernel.
   typedef std::vector<struct sock_filter> Program;
 
+  // Addr represents a logical location within the program being compiled.
+  // Identical code suffixes may be collapsed to a single location.
+  using Addr = Instruction*;

[mdempsky, 2014/11/04 01:31:15]

(As a side note: I'm not particularly attached to this name, if you have
suggestions for an alternative.  I've also been considering "Loc[ation]" and
"Node".)

+
+  // kNullAddr represents the "null" address value; i.e., the reserved
+  // address value guaranteed to not equal any actual address values.
+  static const Addr kNullAddr;
+
   CodeGen();
   ~CodeGen();
 
@@ -67,18 +75,15 @@ class SANDBOX_EXPORT CodeGen {
   // are owned by the CodeGen object. They do not need to be explicitly
   // deleted.
   // For details on the possible parameters refer to <linux/filter.h>
-  Instruction* MakeInstruction(uint16_t code,
-                               uint32_t k,
-                               Instruction* next = nullptr);
-  Instruction* MakeInstruction(uint16_t code,
-                               uint32_t k,
-                               Instruction* jt,
-                               Instruction* jf);
+  Addr MakeInstruction(uint16_t code,
+                       uint32_t k,
+                       Addr jt = kNullAddr,
+                       Addr jf = kNullAddr);

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

These are normally illegal. Overload are better tolerated when they don't have
the same number of arguments.

This is fine with me though since you're modifying existing code, but you could
consider emulating the default arguments with an overload. (Yeah, it seems a bit
silly :/)

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

I've decided to just add a TODO to reconsider the default arguments for now.
There are a few ways I can see to address this, and I'll have an easier way
deciding which one is best once these three CLs have landed.

 
   // Compiles the graph of instructions into a BPF program that can be passed
   // to the kernel. Please note that this function modifies the graph in place
   // and must therefore only be called once per graph.
-  void Compile(Instruction* instructions, Program* program);
+  void Compile(Addr head, Program* program);
 
  private:
   friend class CodeGenUnittestHelper;