Merge remote-tracking branch 'origin/topic/jsiwek/misc-val-tweaks'

* origin/topic/jsiwek/misc-val-tweaks:
  Fix sign-compare compiler warning in coerce_to_record()
  Fix maybe-uninitialized warning in ZVal::ToVal()
  Change RecordVal::GetFieldAs() to use std::vector::operator[]
  Add RecordVal::AssignField() and use it in supervisor code

CHANGES

@@ -1,4 +1,29 @@
 
+4.1.0-dev.434 | 2021-03-29 13:18:18 -0700
+
+  * Fix sign-compare compiler warning in coerce_to_record() (Jon Siwek, Corelight)
+
+  * Fix maybe-uninitialized warning in ZVal::ToVal() (Jon Siwek, Corelight)
+
+  * Change RecordVal::GetFieldAs() to use std::vector::operator[] (Jon Siwek, Corelight)
+
+    Since the method claims it's up to the user to ensure the field exists
+    before calling, the extra bounds-checking done by std::vector::at()
+    isn't needed.
+
+  * Add RecordVal::AssignField() and use it in supervisor code (Jon Siwek, Corelight)
+
+    This is a convenience method to assign a known record field value by
+    field name.  May also be useful to reduce warnings from static analysis
+    (e.g. Coverity) about not checking for negative return values before
+    assigning since that now flows through a [[noreturn]] error path.
+
+  * GH-960: Fix include order of bundled header files (Jon Siwek, Corelight)
+
+    Previously, a system-wide installation of any bundled auxil/ software
+    (like CAF) may get found/included rather than the bundled version and
+    possibly break the build.
+
 4.1.0-dev.427 | 2021-03-27 14:18:16 -0700
 
   * Update bundled CAF to 0.18.2 (Jon Siwek, Corelight)

VERSION

@@ -1 +1 @@
-4.1.0-dev.427
+4.1.0-dev.434

src/Expr.cc

@@ -3890,7 +3890,7 @@ ValPtr RecordCoerceExpr::Fold(Val* v) const
 RecordValPtr coerce_to_record(RecordTypePtr rt, Val* v,
 				const std::vector<int>& map)
 	{
-	auto map_size = map.size();
+	int map_size = map.size();
 	auto val = make_intrusive<RecordVal>(rt);
 	RecordType* val_type = val->GetType()->AsRecordType();
 

src/Val.h

@@ -1152,6 +1152,19 @@ public:
 	void Assign(int field, String* new_val)
 		{ Assign(field, new StringVal(new_val)); }
 
+	/**
+	 * Assign a value of type @c T to a record field of the given name.
+	 * A fatal error occurs if the no such field name exists.
+	 */
+	template <class T>
+	void AssignField(const char* field_name, T&& val)
+		{
+		int idx = GetType()->AsRecordType()->FieldOffset(field_name);
+		if ( idx < 0 )
+			reporter->InternalError("missing record field: %s", field_name);
+		Assign(idx, std::forward<T>(val));
+		}
+
 	/**
 	 * Appends a value to the record's fields.  The caller is responsible
 	 * for ensuring that fields are appended in the correct order and
@@ -1288,33 +1301,33 @@ public:
 		if constexpr ( std::is_same_v<T, BoolVal> ||
 		               std::is_same_v<T, IntVal> ||
 		               std::is_same_v<T, EnumVal> )
-			return record_val->at(field).int_val;
+			return record_val->operator[](field).int_val;
 		else if constexpr ( std::is_same_v<T, CountVal> )
-			return record_val->at(field).uint_val;
+			return record_val->operator[](field).uint_val;
 		else if constexpr ( std::is_same_v<T, DoubleVal> ||
 		                    std::is_same_v<T, TimeVal> ||
 		                    std::is_same_v<T, IntervalVal> )
-			return record_val->at(field).double_val;
+			return record_val->operator[](field).double_val;
 		else if constexpr ( std::is_same_v<T, PortVal> )
 			return val_mgr->Port(record_val->at(field).uint_val);
 		else if constexpr ( std::is_same_v<T, StringVal> )
-			return record_val->at(field).string_val->Get();
+			return record_val->operator[](field).string_val->Get();
 		else if constexpr ( std::is_same_v<T, AddrVal> )
-			return record_val->at(field).addr_val->Get();
+			return record_val->operator[](field).addr_val->Get();
 		else if constexpr ( std::is_same_v<T, SubNetVal> )
-			return record_val->at(field).subnet_val->Get();
+			return record_val->operator[](field).subnet_val->Get();
 		else if constexpr ( std::is_same_v<T, File> )
-			return *(record_val->at(field).file_val);
+			return *(record_val->operator[](field).file_val);
 		else if constexpr ( std::is_same_v<T, Func> )
-			return *(record_val->at(field).func_val);
+			return *(record_val->operator[](field).func_val);
 		else if constexpr ( std::is_same_v<T, PatternVal> )
-			return record_val->at(field).re_val->Get();
+			return record_val->operator[](field).re_val->Get();
 		else if constexpr ( std::is_same_v<T, RecordVal> )
-			return record_val->at(field).record_val;
+			return record_val->operator[](field).record_val;
 		else if constexpr ( std::is_same_v<T, VectorVal> )
-			return record_val->at(field).vector_val;
+			return record_val->operator[](field).vector_val;
 		else if constexpr ( std::is_same_v<T, TableVal> )
-			return record_val->at(field).table_val->Get();
+			return record_val->operator[](field).table_val->Get();
 		else
 			{
 			// It's an error to reach here, although because of
@@ -1329,12 +1342,12 @@ public:
 	T GetFieldAs(int field) const
 		{
 		if constexpr ( std::is_integral_v<T> && std::is_signed_v<T> )
-			return record_val->at(field).int_val;
+			return record_val->operator[](field).int_val;
 		else if constexpr ( std::is_integral_v<T> &&
 					std::is_unsigned_v<T> )
-			return record_val->at(field).uint_val;
+			return record_val->operator[](field).uint_val;
 		else if constexpr ( std::is_floating_point_v<T> )
-			return record_val->at(field).double_val;
+			return record_val->operator[](field).double_val;
 
 		// Note: we could add other types here using type traits,
 		// such as is_same_v<T, std::string>, etc.

src/ZVal.cc

@@ -263,8 +263,9 @@ ValPtr ZVal::ToVal(const TypePtr& t) const
 	case TYPE_TIMER:
 	case TYPE_UNION:
 	case TYPE_VOID:
+	default:
 		v = nullptr;
-		reporter->InternalError("bad ret type return tag");
+		reporter->InternalError("bad type in ZVal::ToVal: %s", type_name(t->Tag()));
 	}
 
 	if ( v )

src/supervisor/Supervisor.cc

@@ -1348,22 +1348,22 @@ RecordValPtr Supervisor::NodeConfig::ToRecord() const
 	{
 	const auto& rt = BifType::Record::Supervisor::NodeConfig;
 	auto rval = make_intrusive<RecordVal>(rt);
-	rval->Assign(rt->FieldOffset("name"), name);
+	rval->AssignField("name", name);
 
 	if ( interface )
-		rval->Assign(rt->FieldOffset("interface"), *interface);
+		rval->AssignField("interface", *interface);
 
 	if ( directory )
-		rval->Assign(rt->FieldOffset("directory"), *directory);
+		rval->AssignField("directory", *directory);
 
 	if ( stdout_file )
-		rval->Assign(rt->FieldOffset("stdout_file"), *stdout_file);
+		rval->AssignField("stdout_file", *stdout_file);
 
 	if ( stderr_file )
-		rval->Assign(rt->FieldOffset("stderr_file"), *stderr_file);
+		rval->AssignField("stderr_file", *stderr_file);
 
 	if ( cpu_affinity )
-		rval->Assign(rt->FieldOffset("cpu_affinity"), *cpu_affinity);
+		rval->AssignField("cpu_affinity", *cpu_affinity);
 
 	auto st = rt->GetFieldType<VectorType>("scripts");
 	auto scripts_val = make_intrusive<VectorVal>(std::move(st));
@@ -1371,11 +1371,11 @@ RecordValPtr Supervisor::NodeConfig::ToRecord() const
 	for ( const auto& s : scripts )
 		scripts_val->Assign(scripts_val->Size(), make_intrusive<StringVal>(s));
 
-	rval->Assign(rt->FieldOffset("scripts"), std::move(scripts_val));
+	rval->AssignField("scripts", std::move(scripts_val));
 
 	auto tt = rt->GetFieldType<TableType>("cluster");
 	auto cluster_val = make_intrusive<TableVal>(std::move(tt));
-	rval->Assign(rt->FieldOffset("cluster"), cluster_val);
+	rval->AssignField("cluster", cluster_val);
 
 	for ( const auto& e : cluster )
 		{
@@ -1385,12 +1385,12 @@ RecordValPtr Supervisor::NodeConfig::ToRecord() const
 		const auto& ept = BifType::Record::Supervisor::ClusterEndpoint;
 		auto val = make_intrusive<RecordVal>(ept);
 
-		val->Assign(ept->FieldOffset("role"), BifType::Enum::Supervisor::ClusterRole->GetEnumVal(ep.role));
-		val->Assign(ept->FieldOffset("host"), make_intrusive<AddrVal>(ep.host));
-		val->Assign(ept->FieldOffset("p"), val_mgr->Port(ep.port, TRANSPORT_TCP));
+		val->AssignField("role", BifType::Enum::Supervisor::ClusterRole->GetEnumVal(ep.role));
+		val->AssignField("host", make_intrusive<AddrVal>(ep.host));
+		val->AssignField("p", val_mgr->Port(ep.port, TRANSPORT_TCP));
 
 		if ( ep.interface )
-			val->Assign(ept->FieldOffset("interface"), *ep.interface);
+			val->AssignField("interface", *ep.interface);
 
 		cluster_val->Assign(std::move(key), std::move(val));
 		}
@@ -1403,10 +1403,10 @@ RecordValPtr SupervisorNode::ToRecord() const
 	const auto& rt = BifType::Record::Supervisor::NodeStatus;
 	auto rval = make_intrusive<RecordVal>(rt);
 
-	rval->Assign(rt->FieldOffset("node"), config.ToRecord());
+	rval->AssignField("node", config.ToRecord());
 
 	if ( pid )
-		rval->Assign(rt->FieldOffset("pid"), pid);
+		rval->AssignField("pid", pid);
 
 	return rval;
 	}
@@ -1458,17 +1458,15 @@ bool SupervisedNode::InitCluster() const
 		auto val = make_intrusive<RecordVal>(cluster_node_type);
 
 		auto node_type = supervisor_role_to_cluster_node_type(ep.role);
-		val->Assign(cluster_node_type->FieldOffset("node_type"), std::move(node_type));
-		val->Assign(cluster_node_type->FieldOffset("ip"), make_intrusive<AddrVal>(ep.host));
-		val->Assign(cluster_node_type->FieldOffset("p"), val_mgr->Port(ep.port, TRANSPORT_TCP));
+		val->AssignField("node_type", std::move(node_type));
+		val->AssignField("ip", make_intrusive<AddrVal>(ep.host));
+		val->AssignField("p", val_mgr->Port(ep.port, TRANSPORT_TCP));
 
 		if ( ep.interface )
-			val->Assign(cluster_node_type->FieldOffset("interface"),
-			            *ep.interface);
+			val->AssignField("interface", *ep.interface);
 
 		if ( manager_name && ep.role != BifEnum::Supervisor::MANAGER )
-			val->Assign(cluster_node_type->FieldOffset("manager"),
-			            *manager_name);
+			val->AssignField("manager", *manager_name);
 
 		cluster_nodes->Assign(std::move(key), std::move(val));
 		}

src/supervisor/supervisor.bif

@@ -86,7 +86,7 @@ function Supervisor::__node%(%): Supervisor::NodeConfig
 		zeek::emit_builtin_error("not a supervised process");
 		const auto& rt = zeek::BifType::Record::Supervisor::NodeConfig;
 		auto rval = zeek::make_intrusive<zeek::RecordVal>(rt);
-		rval->Assign(rt->FieldOffset("name"), "<invalid>");
+		rval->AssignField("name", "<invalid>");
 		return rval;
 		}