Switch plugin::Manager::HookCallFunction() to return IntrusivePtr

The plugin::Plugin side of things is not (yet) changed.

src/Func.cc

@@ -59,7 +59,7 @@ extern	RETSIGTYPE sig_handler(int signo);
 std::vector<CallInfo> call_stack;
 bool did_builtin_init = false;
 
-static const std::pair<bool, Val*> empty_hook_result(false, NULL);
+static const std::pair<bool, IntrusivePtr<Val>> empty_hook_result(false, nullptr);
 
 std::string render_call_stack()
 	{
@@ -214,31 +214,34 @@ void Func::CopyStateInto(Func* other) const
 	other->unique_id = unique_id;
 	}
 
-std::pair<bool, Val*> Func::HandlePluginResult(std::pair<bool, Val*> plugin_result, function_flavor flavor) const
+void Func::CheckPluginResult(bool hooked, const IntrusivePtr<Val>& hook_result,
+                             function_flavor flavor) const
 	{
 	// Helper function factoring out this code from BroFunc:Call() for
 	// better readability.
 
-	if( ! plugin_result.first )
+	if ( ! hooked )
 		{
-		if( plugin_result.second )
+		if ( hook_result )
 			reporter->InternalError("plugin set processed flag to false but actually returned a value");
 
 		// The plugin result hasn't been processed yet (read: fall
 		// into ::Call method).
-		return plugin_result;
+		return;
 		}
 
 	switch ( flavor ) {
 	case FUNC_FLAVOR_EVENT:
-		if( plugin_result.second )
+		if ( hook_result )
-			reporter->InternalError("plugin returned non-void result for event %s", this->Name());
+			reporter->InternalError("plugin returned non-void result for event %s",
+			                        this->Name());
 
 		break;
 
 	case FUNC_FLAVOR_HOOK:
-		if ( plugin_result.second->GetType()->Tag() != TYPE_BOOL )
+		if ( hook_result->GetType()->Tag() != TYPE_BOOL )
-			reporter->InternalError("plugin returned non-bool for hook %s", this->Name());
+			reporter->InternalError("plugin returned non-bool for hook %s",
+			                        this->Name());
 
 		break;
 
@@ -248,21 +251,20 @@ std::pair<bool, Val*> Func::HandlePluginResult(std::pair<bool, Val*> plugin_resu
 
 		if ( (! yt) || yt->Tag() == TYPE_VOID )
 			{
-			if( plugin_result.second )
+			if ( hook_result )
-				reporter->InternalError("plugin returned non-void result for void method %s", this->Name());
+				reporter->InternalError("plugin returned non-void result for void method %s",
+				                        this->Name());
 			}
 
-		else if ( plugin_result.second && plugin_result.second->GetType()->Tag() != yt->Tag() && yt->Tag() != TYPE_ANY)
+		else if ( hook_result && hook_result->GetType()->Tag() != yt->Tag() && yt->Tag() != TYPE_ANY )
 			{
 			reporter->InternalError("plugin returned wrong type (got %d, expecting %d) for %s",
-						plugin_result.second->GetType()->Tag(), yt->Tag(), this->Name());
+			                        hook_result->GetType()->Tag(), yt->Tag(), this->Name());
 			}
 
 		break;
 		}
 	}
-
-	return plugin_result;
 	}
 
 BroFunc::BroFunc(ID* arg_id, IntrusivePtr<Stmt> arg_body, id_list* aggr_inits,
@@ -307,12 +309,14 @@ IntrusivePtr<Val> BroFunc::operator()(const zeek::Args& args, Frame* parent) con
 	if ( sample_logger )
 		sample_logger->FunctionSeen(this);
 
-	std::pair<bool, Val*> plugin_result = PLUGIN_HOOK_WITH_RESULT(HOOK_CALL_FUNCTION, HookCallFunction(this, parent, args), empty_hook_result);
+	auto [hooked, hook_result] = PLUGIN_HOOK_WITH_RESULT(HOOK_CALL_FUNCTION,
+	                                                     HookCallFunction(this, parent, args),
+	                                                     empty_hook_result);
 
-	plugin_result = HandlePluginResult(plugin_result,  Flavor());
+	CheckPluginResult(hooked, hook_result, Flavor());
 
-	if( plugin_result.first )
+	if ( hooked )
-		return {AdoptRef{}, plugin_result.second};
+		return hook_result;
 
 	if ( bodies.empty() )
 		{
@@ -613,12 +617,14 @@ IntrusivePtr<Val> BuiltinFunc::operator()(const zeek::Args& args, Frame* parent)
 	if ( sample_logger )
 		sample_logger->FunctionSeen(this);
 
-	std::pair<bool, Val*> plugin_result = PLUGIN_HOOK_WITH_RESULT(HOOK_CALL_FUNCTION, HookCallFunction(this, parent, args), empty_hook_result);
+	auto [hooked, hook_result] = PLUGIN_HOOK_WITH_RESULT(HOOK_CALL_FUNCTION,
+	                                                     HookCallFunction(this, parent, args),
+	                                                     empty_hook_result);
 
-	plugin_result = HandlePluginResult(plugin_result, FUNC_FLAVOR_FUNCTION);
+	CheckPluginResult(hooked, hook_result, FUNC_FLAVOR_FUNCTION);
 
-	if ( plugin_result.first )
+	if ( hooked )
-		return {AdoptRef{}, plugin_result.second};
+		return hook_result;
 
 	if ( g_trace_state.DoTrace() )
 		{

src/Func.h

@@ -109,8 +109,9 @@ protected:
 	// Copies this function's state into other.
 	void CopyStateInto(Func* other) const;
 
-	// Helper function for handling result of plugin hook.
+	// Helper function for checking result of plugin hook.
-	std::pair<bool, Val*> HandlePluginResult(std::pair<bool, Val*> plugin_result, function_flavor flavor) const;
+	void CheckPluginResult(bool hooked, const IntrusivePtr<Val>& hook_result,
+	                       function_flavor flavor) const;
 
 	std::vector<Body> bodies;
 	IntrusivePtr<Scope> scope;

src/plugin/Manager.cc

@@ -14,6 +14,7 @@
 #include "../Reporter.h"
 #include "../Func.h"
 #include "../Event.h"
+#include "../Val.h"
 #include "../util.h"
 #include "../input.h"
 
@@ -621,7 +622,9 @@ int Manager::HookLoadFile(const Plugin::LoadType type, const string& file, const
 	return rc;
 	}
 
-std::pair<bool, Val*> Manager::HookCallFunction(const Func* func, Frame* parent, const zeek::Args& vecargs) const
+std::pair<bool, IntrusivePtr<Val>>
+Manager::HookCallFunction(const Func* func, Frame* parent,
+                          const zeek::Args& vecargs) const
 	{
 	HookArgumentList args;
 	std::optional<val_list> vargs;
@@ -641,7 +644,7 @@ std::pair<bool, Val*> Manager::HookCallFunction(const Func* func, Frame* parent,
 
 	hook_list* l = hooks[HOOK_CALL_FUNCTION];
 
-	std::pair<bool, Val*> v = std::pair<bool, Val*>(false, NULL);
+	std::pair<bool, Val*> rval{false, nullptr};
 
 	if ( l )
 		{
@@ -657,17 +660,17 @@ std::pair<bool, Val*> Manager::HookCallFunction(const Func* func, Frame* parent,
 			{
 			Plugin* p = (*i).second;
 
-			v = p->HookCallFunction(func, parent, &vargs.value());
+			rval = p->HookCallFunction(func, parent, &vargs.value());
 
-			if ( v.first )
+			if ( rval.first )
 				break;
 			}
 		}
 
 	if ( HavePluginForHook(META_HOOK_POST) )
-		MetaHookPost(HOOK_CALL_FUNCTION, args, HookArgument(v));
+		MetaHookPost(HOOK_CALL_FUNCTION, args, HookArgument(rval));
 
-	return v;
+	return {rval.first, {AdoptRef{}, rval.second}};
 	}
 
 bool Manager::HookQueueEvent(Event* event) const

src/plugin/Manager.h

@@ -245,15 +245,18 @@ public:
 	 *
 	 * @param func The function to be called.
 	 *
+	 * @param parent The frame in which the function is being called.
+	 *
 	 * @param args The function call's arguments; they may be modified by the
 	 * method.
 	 *
-	 * @return If a plugin handled the call, a Val with a +1 reference count
+	 * @return If a plugin handled the call, a Val representing the result
-	 * containing the result value to pass back to the interpreter (for void
+	 * to pass back to the interpreter (for void functions and events,
-	 * functions and events, it may be any Val and must be ignored). If no
+	 * it may be any Val and must be ignored). If no plugin handled the call,
-	 * plugin handled the call, the method returns null.
+	 * the method returns null.
 	 */
-	std::pair<bool, Val*> HookCallFunction(const Func* func, Frame* parent, const zeek::Args& args) const;
+	std::pair<bool, IntrusivePtr<Val>>
+	HookCallFunction(const Func* func, Frame* parent, const zeek::Args& args) const;
 
 	/**
 	 * Hook that filters the queuing of an event.