Merge remote-tracking branch 'origin/topic/timw/lgtm'

- Fixed leak in threading::Field copy-assignment operator

* origin/topic/timw/lgtm:
  Use const-reference in plugin::Manager::MetaHookPost for minor performance gain
  Fix missing assigmnent operator/copy constructor pairings reported by LGTM
  Fix variable shadowing issues reported by LGTM
  Update binpac and broker submodules to fix LGTM findings

auxil/binpac

@@ -1 +1 @@
-Subproject commit 9dd6f1719a53e5bf27e1a292de181bec8556c165
+Subproject commit daaecd4c76317b91d625f4919f7f104e9e6d76fa

src/CCL.cc

@@ -34,8 +34,8 @@ void CCL::Add(int sym)
 	auto sym_p = static_cast<std::intptr_t>(sym);
 
 	// Check to see if the character is already in the ccl.
-	for ( auto sym : *syms )
+	for ( auto sym_entry : *syms )
-		if ( sym == sym_p )
+		if ( sym_entry == sym_p )
 			return;
 
 	syms->push_back(sym_p);

src/CompHash.cc

@@ -187,14 +187,14 @@ char* CompositeHash::SingleValHash(bool type_check, char* kp0,
 				auto rv_i = rv->GetField(i).get();
 
 				Attributes* a = rt->FieldDecl(i)->attrs.get();
-				bool optional = (a && a->Find(ATTR_OPTIONAL));
+				bool optional_attr = (a && a->Find(ATTR_OPTIONAL));
 
-				if ( ! (rv_i || optional) )
+				if ( ! (rv_i || optional_attr) )
 					return nullptr;
 
 				if ( ! (kp = SingleValHash(type_check, kp,
 							   rt->GetFieldType(i).get(),
-							   rv_i, optional)) )
+							   rv_i, optional_attr)) )
 					return nullptr;
 				}
 
@@ -292,9 +292,9 @@ char* CompositeHash::SingleValHash(bool type_check, char* kp0,
 			kp1 = reinterpret_cast<char*>(kp+1);
 			for ( int i = 0; i < lv->Length(); ++i )
 				{
-				Val* v = lv->Idx(i).get();
+				Val* entry_val = lv->Idx(i).get();
-				if ( ! (kp1 = SingleValHash(type_check, kp1, v->GetType().get(), v,
+				if ( ! (kp1 = SingleValHash(type_check, kp1, entry_val->GetType().get(),
-				                            false)) )
+				                            entry_val, false)) )
 					return nullptr;
 				}
 			}
@@ -515,11 +515,11 @@ int CompositeHash::SingleTypeKeySize(Type* bt, const Val* v,
 			for ( int i = 0; i < num_fields; ++i )
 				{
 				Attributes* a = rt->FieldDecl(i)->attrs.get();
-				bool optional = (a && a->Find(ATTR_OPTIONAL));
+				bool optional_attr = (a && a->Find(ATTR_OPTIONAL));
 
 				sz = SingleTypeKeySize(rt->GetFieldType(i).get(),
 						       rv ? rv->GetField(i).get() : nullptr,
-						       type_check, sz, optional,
+						       type_check, sz, optional_attr,
 						       calc_static_size);
 				if ( ! sz )
 					return 0;

src/Debug.cc

@@ -113,14 +113,14 @@ int debug_msg(const char* fmt, ...)
 
 // Trace message output
 
-FILE* TraceState::SetTraceFile(const char* filename)
+FILE* TraceState::SetTraceFile(const char* trace_filename)
 	{
 	FILE* newfile;
 
-	if ( util::streq(filename, "-") )
+	if ( util::streq(trace_filename, "-") )
 		newfile = stderr;
 	else
-		newfile = fopen(filename, "w");
+		newfile = fopen(trace_filename, "w");
 
 	FILE* oldfile = trace_file;
 	if ( newfile )
@@ -129,7 +129,7 @@ FILE* TraceState::SetTraceFile(const char* filename)
 		}
 	else
 		{
-		fprintf(stderr, "Unable to open trace file %s\n", filename);
+		fprintf(stderr, "Unable to open trace file %s\n", trace_filename);
 		trace_file = nullptr;
 		}
 
@@ -326,19 +326,19 @@ static void parse_function_name(vector<ParseLocationRec>& result,
 	else
 		{
 		result.pop_back();
-		ParseLocationRec plr;
+		ParseLocationRec result_plr;
 
-		for ( unsigned int i = 0; i < bodies.size(); ++i )
+		for ( const auto& body : bodies )
 			{
-			get_first_statement(bodies[i].stmts.get(), first, stmt_loc);
+			get_first_statement(body.stmts.get(), first, stmt_loc);
 			if ( ! first )
 				continue;
 
-			plr.type = PLR_FUNCTION;
+			result_plr.type = PLR_FUNCTION;
-			plr.stmt = first;
+			result_plr.stmt = first;
-			plr.filename = stmt_loc.filename;
+			result_plr.filename = stmt_loc.filename;
-			plr.line = stmt_loc.last_line;
+			result_plr.line = stmt_loc.last_line;
-			result.push_back(plr);
+			result.push_back(result_plr);
 			}
 		}
 	}
@@ -369,17 +369,17 @@ vector<ParseLocationRec> parse_location_string(const string& s)
 			parse_function_name(result, plr, s);
 		else
 			{ // file:line
-			string filename = s.substr(0, pos_colon);
+			string policy_filename = s.substr(0, pos_colon);
 			string line_string = s.substr(pos_colon + 1, s.length() - pos_colon);
 
 			if ( ! sscanf(line_string.c_str(), "%d", &plr.line) )
 				plr.type = PLR_UNKNOWN;
 
-			string path(util::find_script_file(filename, util::zeek_path()));
+			string path(util::find_script_file(policy_filename, util::zeek_path()));
 
 			if ( path.empty() )
 				{
-				debug_msg("No such policy file: %s.\n", filename.c_str());
+				debug_msg("No such policy file: %s.\n", policy_filename.c_str());
 				plr.type = PLR_UNKNOWN;
 				return result;
 				}

src/Debug.h

@@ -48,7 +48,7 @@ public:
 	TraceState()	{ dbgtrace = false; trace_file = stderr; }
 
 	// Returns previous filename.
-	FILE* SetTraceFile(const char* filename);
+	FILE* SetTraceFile(const char* trace_filename);
 
 	bool DoTrace() const	{ return dbgtrace; }
 	void TraceOn();

src/IPAddr.h

@@ -33,6 +33,11 @@ struct ConnIDKey {
 		memset(&ip2, 0, sizeof(in6_addr));
 		}
 
+	ConnIDKey(const ConnIDKey& rhs)
+		{
+		*this = rhs;
+		}
+
 	bool operator<(const ConnIDKey& rhs) const { return memcmp(this, &rhs, sizeof(ConnIDKey)) < 0; }
 	bool operator==(const ConnIDKey& rhs) const { return memcmp(this, &rhs, sizeof(ConnIDKey)) == 0; }
 

src/TunnelEncapsulation.h

@@ -76,6 +76,22 @@ public:
 	~EncapsulatingConn()
 		{}
 
+	EncapsulatingConn& operator=(const EncapsulatingConn& other)
+		{
+		if ( this != &other )
+			{
+			src_addr = other.src_addr;
+			dst_addr = other.dst_addr;
+			src_port = other.src_port;
+			dst_port = other.dst_port;
+			proto = other.proto;
+			type = other.type;
+			uid = other.uid;
+			}
+
+		return *this;
+		}
+
 	BifEnum::Tunnel::Type Type() const
 		{ return type; }
 

src/plugin/Manager.cc

@@ -942,7 +942,7 @@ void Manager::MetaHookPre(HookType hook, const HookArgumentList& args) const
 			plugin->MetaHookPre(hook, args);
 	}
 
-void Manager::MetaHookPost(HookType hook, const HookArgumentList& args, HookArgument result) const
+void Manager::MetaHookPost(HookType hook, const HookArgumentList& args, const HookArgument& result) const
 	{
 	if ( hook_list* l = hooks[HOOK_CALL_FUNCTION] )
 		for ( const auto& [hook_type, plugin] : *l )

src/plugin/Manager.h

@@ -414,7 +414,7 @@ private:
 	bool ActivateDynamicPluginInternal(const std::string& name, bool ok_if_not_found, std::vector<std::string>* errors);
 	void UpdateInputFiles();
 	void MetaHookPre(HookType hook, const HookArgumentList& args) const;
-	void MetaHookPost(HookType hook, const HookArgumentList& args, HookArgument result) const;
+	void MetaHookPost(HookType hook, const HookArgumentList& args, const HookArgument& result) const;
 
 	// Plugins that were explicitly requested to be activated, but failed to
 	// load at first.

src/threading/SerialTypes.h

@@ -47,6 +47,22 @@ struct Field {
 		delete [] secondary_name;
 		}
 
+	Field& operator=(const Field& other)
+		{
+		if ( this != &other )
+			{
+			delete [] name;
+			delete [] secondary_name;
+			name = other.name ? util::copy_string(other.name) : nullptr;
+			secondary_name = other.secondary_name ? util::copy_string(other.secondary_name) : nullptr;
+			type = other.type;
+			subtype = other.subtype;
+			optional = other.optional;
+			}
+
+		return *this;
+		}
+
 	/**
 	 * Unserializes a field.
 	 *