From 3cf68302a2fdedee4dd3b3c6b8ebb708116eafb8 Mon Sep 17 00:00:00 2001
From: Tim Wojtulewicz <tim@corelight.com>
Date: Thu, 15 May 2025 17:20:31 -0700
Subject: [PATCH] binpac: Make 'bool' a discrete type and handle it differently
 when generating code

This fixes clang-tidy modernize-use-bool-literals findings in the generated code
---
 tools/binpac/src/pac_case.cc        | 5 ++++-
 tools/binpac/src/pac_externtype.def | 2 +-
 tools/binpac/src/pac_exttype.cc     | 5 ++++-
 tools/binpac/src/pac_exttype.h      | 3 ++-
 tools/binpac/src/pac_type.cc        | 4 ++--
 tools/binpac/src/pac_type.h         | 3 ++-
 6 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/tools/binpac/src/pac_case.cc b/tools/binpac/src/pac_case.cc
index cc629d2253..38f7963dfe 100644
--- a/tools/binpac/src/pac_case.cc
+++ b/tools/binpac/src/pac_case.cc
@@ -284,7 +284,10 @@ void GenCaseStr(ExprList* index_list, Output* out_cc, Env* env, Type* switch_typ
             // We're always using "int" for storage, so ok to just
             // cast into the type used by the switch statement since
             // some unsafe stuff is already checked above.
-            out_cc->println("case ((%s)%d):", switch_type->DataTypeStr().c_str(), index_const);
+            if ( ! switch_type->IsBooleanType() )
+                out_cc->println("case ((%s)%d):", switch_type->DataTypeStr().c_str(), index_const);
+            else
+                out_cc->println("case %s:", index_const == 0 ? "false" : "true");
         }
     }
     else {
diff --git a/tools/binpac/src/pac_externtype.def b/tools/binpac/src/pac_externtype.def
index aeac5a51b0..48475d0d39 100644
--- a/tools/binpac/src/pac_externtype.def
+++ b/tools/binpac/src/pac_externtype.def
@@ -1,4 +1,4 @@
-EXTERNTYPE(bool, bool, NUMBER)
+EXTERNTYPE(bool, bool, BOOLEAN)
 EXTERNTYPE(int, int, NUMBER)
 EXTERNTYPE(double, double, NUMBER)
 EXTERNTYPE(string, string, PLAIN)
diff --git a/tools/binpac/src/pac_exttype.cc b/tools/binpac/src/pac_exttype.cc
index d9acd40a75..7b9970c91e 100644
--- a/tools/binpac/src/pac_exttype.cc
+++ b/tools/binpac/src/pac_exttype.cc
@@ -9,7 +9,8 @@ bool ExternType::DefineValueVar() const { return true; }
 string ExternType::DataTypeStr() const {
     switch ( ext_type_ ) {
         case PLAIN:
-        case NUMBER: return id_->Name();
+        case NUMBER:
+        case BOOLEAN: return id_->Name();
         case POINTER: return string(id_->Name()) + " *";
         default: ASSERT(0); return "";
     }
@@ -31,6 +32,8 @@ void ExternType::GenInitCode(Output* out_cc, Env* env) {
         out_cc->println("%s = 0;", env->LValue(value_var()));
     else if ( IsPointerType() )
         out_cc->println("%s = nullptr;", env->LValue(value_var()));
+    else if ( IsBooleanType() )
+        out_cc->println("%s = false;", env->LValue(value_var()));
 
     Type::GenInitCode(out_cc, env);
 }
diff --git a/tools/binpac/src/pac_exttype.h b/tools/binpac/src/pac_exttype.h
index 1abe5e50dc..8f49332d6b 100644
--- a/tools/binpac/src/pac_exttype.h
+++ b/tools/binpac/src/pac_exttype.h
@@ -10,7 +10,7 @@
 
 class ExternType : public Type {
 public:
-    enum EXTType { PLAIN, NUMBER, POINTER };
+    enum EXTType { PLAIN, NUMBER, POINTER, BOOLEAN };
     ExternType(const ID* id, EXTType ext_type) : Type(EXTERN), id_(id), ext_type_(ext_type) {}
 
     bool DefineValueVar() const override;
@@ -21,6 +21,7 @@ public:
     string EvalMember(const ID* member_id) const override;
     bool IsNumericType() const override { return ext_type_ == NUMBER; }
     bool IsPointerType() const override { return ext_type_ == POINTER; }
+    bool IsBooleanType() const override { return ext_type_ == BOOLEAN; }
 
     void GenInitCode(Output* out_cc, Env* env) override;
 
diff --git a/tools/binpac/src/pac_type.cc b/tools/binpac/src/pac_type.cc
index 7a66f65a87..394d1492f8 100644
--- a/tools/binpac/src/pac_type.cc
+++ b/tools/binpac/src/pac_type.cc
@@ -887,8 +887,8 @@ bool Type::CompatibleTypes(Type* type1, Type* type2) {
 Type* Type::LookUpByID(ID* id) {
     // 1. Is it a pre-defined type?
     string name = id->Name();
-    if ( type_map_.find(name) != type_map_.end() ) {
-        return type_map_[name]->Clone();
+    if ( auto it = type_map_.find(name); it != type_map_.end() ) {
+        return it->second->Clone();
     }
 
     // 2. Is it a simple declared type?
diff --git a/tools/binpac/src/pac_type.h b/tools/binpac/src/pac_type.h
index f2cf137706..a4bf2e8eca 100644
--- a/tools/binpac/src/pac_type.h
+++ b/tools/binpac/src/pac_type.h
@@ -110,7 +110,7 @@ public:
     // is numeric or pointer)
     string DataTypeConstRefStr() const {
         string data_type = DataTypeStr();
-        if ( ! IsPointerType() && ! IsNumericType() )
+        if ( ! IsPointerType() && ! IsNumericType() && ! IsBooleanType() )
             data_type += " const&";
         return data_type;
     }
@@ -138,6 +138,7 @@ public:
 
     virtual bool IsPointerType() const = 0;
     virtual bool IsNumericType() const { return false; }
+    virtual bool IsBooleanType() const { return false; }
     bool IsEmptyType() const;
 
     ////////////////////////////////////////