From 03c3d743c0db876d95f6a7b1849ac87aea2f680b Mon Sep 17 00:00:00 2001
From: Alan Jowett <alan.jowett@microsoft.com>
Date: Thu, 7 Nov 2024 09:05:29 -0800
Subject: [PATCH] PR feedback

Signed-off-by: Alan Jowett <alan.jowett@microsoft.com>
---
 src/config.hpp           | 21 ++++++++++++++++++---
 src/crab/cfg.hpp         |  2 +-
 src/test/ebpf_yaml.cpp   |  4 ++--
 src/test/ebpf_yaml.hpp   |  2 +-
 src/test/test_verify.cpp |  4 ++--
 5 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/src/config.hpp b/src/config.hpp
index c33ff5f37..b109a35c8 100644
--- a/src/config.hpp
+++ b/src/config.hpp
@@ -5,10 +5,11 @@
 #include "crab/cfg.hpp"
 
 struct ebpf_verifier_options_t {
+    // Options that control how the control flow graph is built.
     prepare_cfg_options cfg_opts;
+
+    // True to assume prior failed assertions are true and continue verification.
     bool assume_assertions = false;
-    bool print_invariants = false;
-    bool print_failures = false;
 
     // False to use actual map fd's, true to use mock fd's.
     bool mock_map_fds = true;
@@ -16,11 +17,25 @@ struct ebpf_verifier_options_t {
     // True to do additional checks for some things that would fail at runtime.
     bool strict = false;
 
-    bool print_line_info = false;
+    // True to allow division by zero and assume BPF ISA defined semantics.
     bool allow_division_by_zero = true;
+
+    // Setup the entry constraints for a BPF program.
     bool setup_constraints = true;
+
+    // True if the ELF file is built on a big endian system.
     bool big_endian = false;
 
+    // Print the invariants for each basic block.
+    bool print_invariants = false;
+
+    // Print failures that occur during verification.
+    bool print_failures = false;
+
+    // When printing the control flow graph, print the line number of each instruction.
+    bool print_line_info = false;
+
+    // Print the BTF types in JSON format.
     bool dump_btf_types_json = false;
 };
 
diff --git a/src/crab/cfg.hpp b/src/crab/cfg.hpp
index 856a5975b..f190df82b 100644
--- a/src/crab/cfg.hpp
+++ b/src/crab/cfg.hpp
@@ -623,7 +623,7 @@ struct prepare_cfg_options {
     bool must_have_exit = true;
 };
 
-cfg_t prepare_cfg(const InstructionSeq& prog, const program_info& info, const prepare_cfg_options& options = {});
+cfg_t prepare_cfg(const InstructionSeq& prog, const program_info& info, const prepare_cfg_options& options);
 
 void explicate_assertions(cfg_t& cfg, const program_info& info);
 std::vector<Assert> get_assertions(Instruction ins, const program_info& info, const std::optional<label_t>& label);
diff --git a/src/test/ebpf_yaml.cpp b/src/test/ebpf_yaml.cpp
index 7928cd93a..018f5ee92 100644
--- a/src/test/ebpf_yaml.cpp
+++ b/src/test/ebpf_yaml.cpp
@@ -168,7 +168,7 @@ static InstructionSeq raw_cfg_to_instruction_seq(const vector<std::tuple<string,
 }
 
 static ebpf_verifier_options_t raw_options_to_options(const std::set<string>& raw_options) {
-    ebpf_verifier_options_t options;
+    ebpf_verifier_options_t options{};
 
     // Use ~simplify for YAML tests unless otherwise specified.
     options.cfg_opts.simplify = false;
@@ -355,7 +355,7 @@ ConformanceTestResult run_conformance_test_case(const std::vector<std::byte>& me
 
     auto& prog = std::get<InstructionSeq>(prog_or_error);
 
-    ebpf_verifier_options_t options;
+    ebpf_verifier_options_t options{};
     if (debug) {
         print(prog, std::cout, {});
         options.print_failures = true;
diff --git a/src/test/ebpf_yaml.hpp b/src/test/ebpf_yaml.hpp
index 431171356..b427c5114 100644
--- a/src/test/ebpf_yaml.hpp
+++ b/src/test/ebpf_yaml.hpp
@@ -9,7 +9,7 @@
 
 struct TestCase {
     std::string name;
-    ebpf_verifier_options_t options;
+    ebpf_verifier_options_t options{};
     string_invariant assumed_pre_invariant;
     InstructionSeq instruction_seq;
     string_invariant expected_post_invariant;
diff --git a/src/test/test_verify.cpp b/src/test/test_verify.cpp
index dbeb0c5d0..2e7dccae1 100644
--- a/src/test/test_verify.cpp
+++ b/src/test/test_verify.cpp
@@ -598,7 +598,7 @@ TEST_CASE("multithreading", "[verify][multithreading]") {
     auto prog_or_error1 = unmarshal(raw_prog1);
     auto prog1 = std::get_if<InstructionSeq>(&prog_or_error1);
     REQUIRE(prog1 != nullptr);
-    const cfg_t cfg1 = prepare_cfg(*prog1, raw_prog1.info);
+    const cfg_t cfg1 = prepare_cfg(*prog1, raw_prog1.info, {});
 
     auto raw_progs2 = read_elf("ebpf-samples/bpf_cilium_test/bpf_netdev.o", "2/2", {}, &g_ebpf_platform_linux);
     REQUIRE(raw_progs2.size() == 1);
@@ -606,7 +606,7 @@ TEST_CASE("multithreading", "[verify][multithreading]") {
     auto prog_or_error2 = unmarshal(raw_prog2);
     auto prog2 = std::get_if<InstructionSeq>(&prog_or_error2);
     REQUIRE(prog2 != nullptr);
-    const cfg_t cfg2 = prepare_cfg(*prog2, raw_prog2.info);
+    const cfg_t cfg2 = prepare_cfg(*prog2, raw_prog2.info, {});
 
     bool res1, res2;
     std::thread a(test_analyze_thread, &cfg1, &raw_prog1.info, &res1);