PR feedback

Signed-off-by: Alan Jowett <alan.jowett@microsoft.com>

Author: Alan Jowett

tests/sample/unsafe/invalid_maps2.c

@@ -149,7 +149,7 @@ BindMonitor(bind_md_t* ctx)
     uint32_t* dummy_value = bpf_map_lookup_elem(&dummy_map, &dummy_key);
 
     if (!dummy_value) {
-        // Callee0 is at index 0
+        // Callee0 is at index 0.
         int index = 0;
         bpf_tail_call(ctx, &prog_array_map, index);
     }
@@ -165,7 +165,7 @@ BindMonitor_Callee0(bind_md_t* ctx)
     uint32_t* dummy_value = bpf_map_lookup_elem(&dummy_map, &dummy_key);
 
     if (!dummy_value) {
-        // Callee1 is at index 1
+        // Callee1 is at index 1.
         int index = 1;
         bpf_tail_call(ctx, &prog_array_map, index);
     }

tests/stress/km/stress_tests_km.cpp

@@ -30,9 +30,9 @@ static const std::map<std::string, test_program_attributes> _test_program_info =
 struct object_table_entry
 {
     std::unique_ptr<std::mutex> lock{nullptr};
-    _Guarded_by_(lock) bool available{true};
-    _Guarded_by_(lock) bpf_object_ptr object{nullptr};
-    _Guarded_by_(lock) bool loaded{false};
+    _Guarded_by_(lock) bool available { true };
+    _Guarded_by_(lock) bpf_object_ptr object { nullptr };
+    _Guarded_by_(lock) bool loaded { false };
     bool attach{false};
 
     // The following fields are for debugging this test itself.
@@ -353,7 +353,7 @@ _do_creator_work(thread_context& context, std::time_t endtime_seconds)
                 // to force a race between 'load', 'attach' and 'destroy'.
                 std::lock_guard lock(*entry.lock.get());
 
-                // Make sure entry is _still_ free (some other creator thread may have grabbed it)
+                // Make sure entry is _still_ free (some other creator thread may have grabbed it).
                 if (!entry.available) {
                     continue;
                 }
@@ -747,13 +747,13 @@ wait_and_verify_test_threads(
     std::vector<std::thread>& extension_restart_thread_table,
     std::vector<thread_context>& extension_restart_thread_context_table)
 {
-    // Wait for all test threads
+    // Wait for all test threads.
     LOG_VERBOSE("waiting on {} test threads...", thread_table.size());
     for (auto& t : thread_table) {
         t.join();
     }
 
-    // Wait for all extension restart threads
+    // Wait for all extension restart threads.
     if (test_control_info.extension_restart_enabled) {
         LOG_VERBOSE("waiting on {} extension restart threads...", extension_restart_thread_table.size());
         for (auto& t : extension_restart_thread_table) {
@@ -835,7 +835,7 @@ _mt_prog_load_stress_test(ebpf_execution_type_t program_type, const test_control
                     context_entry.file_name = _make_unique_file_copy(program_attribs.native_file_name);
                 } else {
 
-                    // Use the same file name for all 'creator' threads
+                    // Use the same file name for all 'creator' threads.
                     context_entry.file_name = program_attribs.native_file_name;
                 }
             } else {
@@ -1042,7 +1042,7 @@ _invoke_test_thread_function(thread_context& context)
         exit(-1);
     }
 
-    // Set the timeout for connect attempts
+    // Set the timeout for connect attempts.
     timeval timeout;
     timeout.tv_sec = 5; // 5 seconds
     timeout.tv_usec = 0;
@@ -1974,7 +1974,7 @@ _mt_load_stress_test_with_restart_timing(
     if (test_control_info.extension_restart_enabled && !start_restart_before_load) {
         // Wait for programs to load, then start extension restart.
         LOG_INFO("Waiting for programs to load, then starting extension restart");
-        std::this_thread::sleep_for(std::chrono::seconds(5)); // Give programs time to load and attach
+        std::this_thread::sleep_for(std::chrono::seconds(5)); // Give programs time to load and attach.
         configure_extension_restart(
             test_control_info,
             extension_names,
@@ -2032,9 +2032,9 @@ _mt_invoke_stress_test_multiple_programs(ebpf_execution_type_t program_type, con
         }
 
         context_entry.program_name = "cgroup_sock_addr";
-        context_entry.role = thread_role_type::CREATOR; // All threads are creators for this test
+        context_entry.role = thread_role_type::CREATOR; // All threads are creators for this test.
         context_entry.thread_index = i;
-        context_entry.compartment_id = i + 1; // Unique compartment IDs
+        context_entry.compartment_id = i + 1; // Unique compartment IDs.
         context_entry.duration_minutes = test_control_info.duration_minutes;
         context_entry.extension_restart_enabled = test_control_info.extension_restart_enabled;
     }

tests/unit/wer_report_test_wrapper.cpp

@@ -45,7 +45,7 @@ AddVectoredExceptionHandler_test(_In_ unsigned long first, _In_ PVECTORED_EXCEPT
 unsigned long SetThreadStackGuarantee_test_stack_size_in_bytes = 0;
 
 // Use BOOL to pass "SetThreadStackGuarantee"
-// defined in windows "processthreadapi.h" file
+// defined in windows "processthreadapi.h" file.
 BOOL
 SetThreadStackGuarantee_test(_Inout_ unsigned long* stack_size_in_bytes)
 {
@@ -209,7 +209,7 @@ TEST_CASE("wer_report_failure", "[wer_report]")
     WerReportAddDump_test_called = false;
     WerReportSubmit_test_called = false;
 
-    // Fail WerReportCreate
+    // Fail WerReportCreate.
     REQUIRE(AddVectoredExceptionHandler_test_handler(&exception_pointers) == EXCEPTION_CONTINUE_SEARCH);
     REQUIRE(WerReportCreate_test_called);
     REQUIRE(!WerReportAddDump_test_called);
@@ -222,7 +222,7 @@ TEST_CASE("wer_report_failure", "[wer_report]")
     WerReportAddDump_test_called = false;
     WerReportSubmit_test_called = false;
 
-    // Fail WerReportAddDump
+    // Fail WerReportAddDump.
     REQUIRE(AddVectoredExceptionHandler_test_handler(&exception_pointers) == EXCEPTION_CONTINUE_SEARCH);
     REQUIRE(WerReportCreate_test_called);
     REQUIRE(WerReportAddDump_test_called);
@@ -235,7 +235,7 @@ TEST_CASE("wer_report_failure", "[wer_report]")
     WerReportAddDump_test_called = false;
     WerReportSubmit_test_called = false;
 
-    // Fail WerReportSubmit
+    // Fail WerReportSubmit.
     REQUIRE(AddVectoredExceptionHandler_test_handler(&exception_pointers) == EXCEPTION_CONTINUE_SEARCH);
     REQUIRE(WerReportCreate_test_called);
     REQUIRE(WerReportAddDump_test_called);

tools/bpf2c/bpf2c.cpp

@@ -104,7 +104,7 @@ get_program_info_type_hash(const std::vector<int32_t>& actual_helper_ids, const
                 helper_id_ordering[helper_id] = index;
             }
         }
-        // Hash helper ids in increasing helper_id order
+        // Hash helper ids in increasing helper_id order.
         for (auto [helper_id, index] : helper_id_ordering) {
             hash_t::append_byte_range(
                 byte_range, program_info->program_type_specific_helper_prototype[index].helper_id);

tools/bpf2c/bpf2c_dll.c

@@ -4,7 +4,7 @@
 #include "bpf2c.h"
 
 #include <stdio.h>
-#define WIN32_LEAN_AND_MEAN // Exclude rarely-used stuff from Windows headers
+#define WIN32_LEAN_AND_MEAN // Exclude rarely-used stuff from Windows headers.
 #include <windows.h>
 
 #define metadata_table ___METADATA_TABLE___##_metadata_table

tools/bpf2c/bpf_code_generator.cpp

@@ -120,7 +120,7 @@ static const std::string _predicate_format_string[] = {
 
 #define ADD_OPCODE(X) {static_cast<uint8_t>(X), std::string(#X)}
 
-// remove EBPF_ATOMIC_ prefix
+// Remove EBPF_ATOMIC_ prefix.
 #define ADD_ATOMIC_OPCODE(X) {static_cast<int32_t>(X), std::string(#X).substr(12)}
 
 static std::map<int32_t, std::string> _atomic_opcode_name_strings = {
@@ -478,7 +478,7 @@ bpf_code_generator::visit_symbols(symbol_visitor_t visitor, const unsafe_string&
             throw bpf_code_generator_exception("invalid symbol value");
         }
 
-        // Check for overflow of value + size
+        // Check for overflow of value + size.
         if ((value + size) < value) {
             throw bpf_code_generator_exception("invalid symbol value");
         }
@@ -1076,7 +1076,7 @@ bpf_code_generator::bpf_code_generator_program::encode_instructions(
     auto effective_program_name = !program_name.empty() ? program_name : elf_section_name;
     auto helper_array_prefix = "runtime_context->helper_data[{}]";
 
-    // Encode instructions
+    // Encode instructions.
     for (size_t i = 0; i < program_output.size(); i++) {
         auto& output = program_output[i];
         auto& inst = output.instruction;

tools/netsh/dllmain.c

@@ -32,15 +32,15 @@ DllMain(HMODULE moduleHandle, unsigned long reasonForCall, void* reserved)
     return TRUE;
 }
 
-// Verbs
+// Verbs.
 #define CMD_GROUP_ADD L"add"
 #define CMD_GROUP_DELETE L"delete"
 #define CMD_GROUP_PIN L"pin"
 #define CMD_GROUP_SET L"set"
 #define CMD_GROUP_SHOW L"show"
 #define CMD_GROUP_UNPIN L"unpin"
 
-// Nouns
+// Nouns.
 #define CMD_EBPF_SHOW_DISASSEMBLY L"disassembly"
 #define CMD_EBPF_SHOW_LINKS L"links"
 #define CMD_EBPF_SHOW_MAPS L"maps"
@@ -87,25 +87,25 @@ CMD_ENTRY g_EbpfShowCommandTable[] = {
 #else
 typedef struct _CMD_ENTRY_ORIGINAL
 {
-    _Field_z_ const wchar_t* pwszCmdToken; // The token for the command
-    PFN_HANDLE_CMD pfnCmdHandler;          // The function which handles this command
-    unsigned long dwShortCmdHelpToken;     // The short help message
+    _Field_z_ const wchar_t* pwszCmdToken; // The token for the command.
+    PFN_HANDLE_CMD pfnCmdHandler;          // The function which handles this command.
+    unsigned long dwShortCmdHelpToken;     // The short help message.
     unsigned long
-        dwCmdHlpToken; // The message to display if the only thing after the command is a help token (HELP, /?, -?, ?)
-    unsigned long dwFlags;              // Flags (see CMD_FLAGS_xxx above)
-    PNS_OSVERSIONCHECK pOsVersionCheck; // Check for the version of the OS this command can run against
+        dwCmdHlpToken; // The message to display if the only thing after the command is a help token (HELP, /?, -?, ?).
+    unsigned long dwFlags;              // Flags (see CMD_FLAGS_xxx above).
+    PNS_OSVERSIONCHECK pOsVersionCheck; // Check for the version of the OS this command can run against.
 } CMD_ENTRY_ORIGINAL, *PCMD_ENTRY_ORIGINAL;
 #define CREATE_CMD_ENTRY_ORIGINAL(t, f) {CMD_##t, f, HLP_##t, HLP_##t##_EX, CMD_FLAG_PRIVATE, NULL}
 
 typedef struct _CMD_ENTRY_LONG
 {
-    _Field_z_ const wchar_t* pwszCmdToken; // The token for the command
-    PFN_HANDLE_CMD pfnCmdHandler;          // The function which handles this command
-    unsigned long dwShortCmdHelpToken;     // The short help message
+    _Field_z_ const wchar_t* pwszCmdToken; // The token for the command.
+    PFN_HANDLE_CMD pfnCmdHandler;          // The function which handles this command.
+    unsigned long dwShortCmdHelpToken;     // The short help message.
     unsigned long
-        dwCmdHlpToken; // The message to display if the only thing after the command is a help token (HELP, /?, -?, ?)
-    unsigned long dwFlags;              // Flags (see CMD_FLAGS_xxx above)
-    PNS_OSVERSIONCHECK pOsVersionCheck; // Check for the version of the OS this command can run against
+        dwCmdHlpToken; // The message to display if the only thing after the command is a help token (HELP, /?, -?, ?).
+    unsigned long dwFlags;              // Flags (see CMD_FLAGS_xxx above).
+    PNS_OSVERSIONCHECK pOsVersionCheck; // Check for the version of the OS this command can run against.
     void* pfnCustomHelpFn;
 } CMD_ENTRY_LONG, *PCMD_ENTRY_LONG;
 #define CREATE_CMD_ENTRY_LONG(t, f) {CMD_##t, f, HLP_##t, HLP_##t##_EX, CMD_FLAG_PRIVATE, NULL, NULL}

undocked/tests/sample/ext/drv/sample_ext_drv.c

@@ -124,8 +124,7 @@ _sample_ebpf_ext_driver_initialize_objects(
     status = WdfDeviceCreate(&device_initialize, WDF_NO_OBJECT_ATTRIBUTES, device);
 
     if (!NT_SUCCESS(status)) {
-        // Do not free if any other call
-        // after WdfDeviceCreate fails.
+        // Do not free if any other call after WdfDeviceCreate fails.
         WdfDeviceInitFree(device_initialize);
         device_initialize = NULL;
         goto Exit;