Incremental commit: implementing a wrapper for the Val class.

Just a checkpoint: need to add / update tests to make sure things work as expected.  Should build / pass core btests, though.

src/Func.cc

@@ -48,6 +48,8 @@
 #include "Reporter.h"
 #include "plugin/Manager.h"
 
+using plugin::ValWrapper;
+
 extern	RETSIGTYPE sig_handler(int signo);
 
 const Expr* calling_expr = 0;
@@ -245,21 +247,40 @@ TraversalCode Func::Traverse(TraversalCallback* cb) const
 	HANDLE_TC_STMT_POST(tc);
 	}
 
-Val* Func::HandlePluginResult(Val* plugin_result, val_list* args, function_flavor flavor) const
+ValWrapper* Func::HandlePluginResult(ValWrapper* plugin_result, val_list* args, function_flavor flavor) const
 	{
-	// Helper function factoring out this code from BroFunc:Call() for better
-	// readability.
+    // We either have not received a plugin result, or the plugin result hasn't been processed (read: fall into ::Call method)
+    if(!plugin_result)
+        return NULL;
+
+    if(!plugin_result->processed)
+        {
+        if(plugin_result->value)
+            {
+            Unref(plugin_result->value);
+            plugin_result->value = NULL;
+            }
+        delete plugin_result;
+        return NULL;
+        }
 
 	switch ( flavor ) {
 	case FUNC_FLAVOR_EVENT:
-		Unref(plugin_result);
-		plugin_result = 0;
+        if(plugin_result->value)
+            {
+            char sbuf[1024];
+            snprintf(sbuf, 1024, "plugin returned non-void result for event %s", this->Name());
+            reporter->InternalError(sbuf);
+            }
 		break;
 
 	case FUNC_FLAVOR_HOOK:
-		if ( plugin_result->Type()->Tag() != TYPE_BOOL )
-			reporter->InternalError("plugin returned non-bool for hook");
-
+		if ( plugin_result->value->Type()->Tag() != TYPE_BOOL )
+		    {
+            char sbuf[1024];
+            snprintf(sbuf, 1024, "plugin returned non-bool for hook %s", this->Name());
+        	reporter->InternalError(sbuf);
+            }
 		break;
 
 	case FUNC_FLAVOR_FUNCTION:
@@ -268,34 +289,15 @@ Val* Func::HandlePluginResult(Val* plugin_result, val_list* args, function_flavo
 
 		if ( (! yt) || yt->Tag() == TYPE_VOID )
 			{
-			Unref(plugin_result);
-			plugin_result = NULL;
-			}
-
-		else
-			{
-            /*
-            FIXME: I know this probably isn't a good idea, but what's the better solution?
-
-            Hack: we want a way to force a NULL return in certain cases (e.g. function delayed).  Since no function should ever reasonably return
-            an error, we use the error type to represent this case.
-
-            Note that re-using a type that a function could reasonably return breaks down in the case of e.g. a delayed function, where the function
-            will have a very specific type but still return NULL because things have not yet been evaluated.  Thus, if the delayed method returns a
-            bool, and our garbage return value is a bool, then how do we know whether or not the Val* returned by the function is actually meaningful
-            in the general case?
-            */
-            if ( plugin_result->Type()->Tag() == TYPE_ERROR )
-                {
-                Unref(plugin_result);
-                plugin_result = NULL;
-                }
-			else if ( plugin_result->Type()->Tag() != yt->Tag() && yt->Tag() != TYPE_ANY)
-                {
             char sbuf[1024];
-                snprintf(sbuf, 1024, "plugin returned wrong type (got %d, expecting %d) for %s", plugin_result->Type()->Tag(), yt->Tag(), this->Name());
+            snprintf(sbuf, 1024, "plugin returned non-void result for void method %s", this->Name());
 			reporter->InternalError(sbuf);
             }
+		else if ( plugin_result->value->Type()->Tag() != yt->Tag() && yt->Tag() != TYPE_ANY)
+			{
+            char sbuf[1024];
+            snprintf(sbuf, 1024, "plugin returned wrong type (got %d, expecting %d) for %s", plugin_result->value->Type()->Tag(), yt->Tag(), this->Name());
+			reporter->InternalError(sbuf);
             }
 
 		break;
@@ -351,10 +353,15 @@ Val* BroFunc::Call(val_list* args, Frame* parent) const
 	if ( sample_logger )
 		sample_logger->FunctionSeen(this);
 
-	Val* plugin_result = PLUGIN_HOOK_WITH_RESULT(HOOK_CALL_FUNCTION, HookCallFunction(this, parent, args), 0);
+	ValWrapper* plugin_result = PLUGIN_HOOK_WITH_RESULT(HOOK_CALL_FUNCTION, HookCallFunction(this, parent, args), 0);
 
+    plugin_result = HandlePluginResult(plugin_result, args, Flavor());
     if(plugin_result)
-		return HandlePluginResult(plugin_result, args, Flavor());
+        {
+        Val *result = plugin_result->value;
+        delete plugin_result;
+        return result;
+        }
 
 	if ( bodies.empty() )
 		{
@@ -568,10 +575,15 @@ Val* BuiltinFunc::Call(val_list* args, Frame* parent) const
 	if ( sample_logger )
 		sample_logger->FunctionSeen(this);
 
-	Val* plugin_result = PLUGIN_HOOK_WITH_RESULT(HOOK_CALL_FUNCTION, HookCallFunction(this, parent, args), 0);
+	ValWrapper* plugin_result = PLUGIN_HOOK_WITH_RESULT(HOOK_CALL_FUNCTION, HookCallFunction(this, parent, args), 0);
 
+    plugin_result = HandlePluginResult(plugin_result, args, FUNC_FLAVOR_FUNCTION);
     if(plugin_result)
-		return HandlePluginResult(plugin_result, args, FUNC_FLAVOR_FUNCTION);
+        {
+        Val *result = plugin_result->value;
+        delete plugin_result;
+        return result;
+        }
 
 	if ( g_trace_state.DoTrace() )
 		{

src/Func.h

@@ -14,6 +14,9 @@ class Stmt;
 class Frame;
 class ID;
 class CallExpr;
+namespace plugin {
+    struct ValWrapper;
+}
 
 class Func : public BroObj {
 public:
@@ -71,7 +74,7 @@ protected:
 	Func();
 
 	// Helper function for handling result of plugin hook.
-	Val* HandlePluginResult(Val* plugin_result, val_list* args, function_flavor flavor) const;
+	plugin::ValWrapper* HandlePluginResult(plugin::ValWrapper* plugin_result, val_list* args, function_flavor flavor) const;
 
 	DECLARE_ABSTRACT_SERIAL(Func);
 

src/plugin/Manager.cc

@@ -559,7 +559,7 @@ int Manager::HookLoadFile(const string& file)
 	return rc;
 	}
 
-Val* Manager::HookCallFunction(const Func* func, Frame* parent, val_list* vargs) const
+ValWrapper* Manager::HookCallFunction(const Func* func, Frame* parent, val_list* vargs) const
 	{
 	HookArgumentList args;
 
@@ -573,7 +573,7 @@ Val* Manager::HookCallFunction(const Func* func, Frame* parent, val_list* vargs)
 
 	hook_list* l = hooks[HOOK_CALL_FUNCTION];
 
-	Val* v = 0;
+	ValWrapper* v = 0;
 
 	if ( l )
 		for ( hook_list::iterator i = l->begin(); i != l->end(); ++i )
@@ -582,7 +582,7 @@ Val* Manager::HookCallFunction(const Func* func, Frame* parent, val_list* vargs)
 
 			v = p->HookCallFunction(func, parent, vargs);
 
-			if ( v )
+			if ( v && v-> processed)
 				break;
 			}
 

src/plugin/Manager.h

@@ -244,7 +244,7 @@ public:
 	 * functions and events, it may be any Val and must be ignored). If no
 	 * plugin handled the call, the method returns null.
 	 */
-	Val* HookCallFunction(const Func* func, Frame *parent, val_list* args) const;
+	ValWrapper* HookCallFunction(const Func* func, Frame *parent, val_list* args) const;
 
 	/**
 	 * Hook that filters the queuing of an event.

src/plugin/Plugin.cc

@@ -106,6 +106,21 @@ void HookArgument::Describe(ODesc* d) const
 			d->Add("<null>");
 		break;
 
+    case WRAPPED_VAL:
+        if ( arg.wrapper )
+            {
+            d->Add("wrapped(");
+            if(arg.wrapper->value)
+                {
+                arg.wrapper->value->Describe(d);
+                }
+            else
+                d->Add("<null>");
+            d->Add(")");
+            }
+
+        break;
+
 	case VAL_LIST:
 		if ( arg.vals )
 			{
@@ -271,7 +286,7 @@ int Plugin::HookLoadFile(const std::string& file, const std::string& ext)
 	return -1;
 	}
 
-Val* Plugin::HookCallFunction(const Func* func, Frame *parent, val_list* args)
+ValWrapper* Plugin::HookCallFunction(const Func* func, Frame *parent, val_list* args)
 	{
 	return 0;
 	}

src/plugin/Plugin.h

@@ -25,6 +25,34 @@ class Manager;
 class Component;
 class Plugin;
 
+/**
+ * In certain cases, functions may have well-defined return types but still return NULL values (e.g. delayed functions, opaque types).
+ * Thus, it's necessary to explicitly define whether or not a plugin has handled a function in addition to recording the value it has
+ * returned.
+ *
+ * Plugins' function handlers return a result of this type.
+ */
+struct ValWrapper {
+    Val* value;       //< value being wrapped by this object
+    bool processed;   //< true if execution should *STOP* (read: the plugin is replacing a method), and false if execution should *CONTINUE* (read: bro should execute a method)
+
+    /**
+    Wrapper for a specific value.  If we're setting a value, we assume we've processed something.
+    
+    @param value value to be wrapped
+    */
+    ValWrapper(Val* value)
+    : value(value), processed(true) { }
+
+    /**
+    Wrapper for a specific value.  If we're setting 'processed', we assume there's a reason we're not setting a Val and set that to NULL.
+    
+    @param processed whether or not an execution of a function was handled by the plugin
+    */
+    ValWrapper(bool processed)
+    : value(NULL), processed(processed) { }
+};
+
 /**
  * Hook types that a plugin may define. Each label maps to the corresponding
  * virtual method in \a Plugin.
@@ -155,7 +183,7 @@ public:
 	 * Type of the argument.
 	 */
 	enum Type {
-		BOOL, DOUBLE, EVENT, FUNC, INT, STRING, VAL, VAL_LIST, VOID, VOIDP,
+		BOOL, DOUBLE, EVENT, FUNC, INT, STRING, VAL, WRAPPED_VAL, VAL_LIST, VOID, VOIDP,
 	};
 
 	/**
@@ -208,6 +236,11 @@ public:
 	 */
 	HookArgument(void* p)	{ type = VOIDP; arg.voidp = p; }
 
+    /**
+     * Constructor with a ValWrapper argument.
+     */
+    HookArgument(ValWrapper* a)   { type = WRAPPED_VAL; arg.wrapper = a; }
+
 	/**
 	 * Returns the value for a boolen argument. The argument's type must
 	 * match accordingly.
@@ -250,6 +283,12 @@ public:
 	 */
 	const Val* AsVal() const	{ assert(type == VAL); return arg.val; }
 
+    /**
+     * Returns the value for a Bro wrapped value argument.  The argument's type must
+     * match accordingly.
+     */
+    const ValWrapper* AsValWrapper() const { assert(type == VAL_WRAPPER); return arg.wrapper; }
+
 	/**
 	 * Returns the value for a list of Bro values argument. The argument's type must
 	 * match accordingly.
@@ -283,6 +322,7 @@ private:
 		const Func* func;
 		int int_;
 		const Val* val;
+        const ValWrapper* wrapper;
 		const val_list* vals;
 		const void* voidp;
 	} arg;
@@ -567,13 +607,14 @@ protected:
 	 * in place as long as it ensures matching types and correct reference
 	 * counting.
 	 *
-	 * @return If the plugin handled the call, a Val with +1 reference
-	 * count containixnmg the result value to pass back to the interpreter
-	 * (for void functions and events any \a Val is fine; it will be
-	 * ignored; best to use a \c TYPE_ANY). If the plugin did not handle
-	 * the call, it must return null.
+	 * @return If the plugin handled the call, a ValWrapper with the
+     * processed flag set to true, and a value set on the object with 
+     * a+1 reference count containing the result value to pass back to the 
+     * interpreter.  If the plugin did not handle the call, it may either
+     * return NULL *or* return a ValWrapper with the processed flag set to
+     * 'false'.
 	 */
-	virtual Val* HookCallFunction(const Func* func, Frame *parent, val_list* args);
+	virtual ValWrapper* HookCallFunction(const Func* func, Frame *parent, val_list* args);
 
 	/**
 	 * Hook into raising events. Whenever the script interpreter is about

testing/btest/plugins/hooks-plugin/src/Plugin.cc

@@ -48,7 +48,7 @@ int Plugin::HookLoadFile(const std::string& file, const std::string& ext)
 	return -1;
 	}
 
-Val* Plugin::HookCallFunction(const Func* func, val_list* args)
+Val* Plugin::HookCallFunction(const Func* func, Frame* frame, val_list* args)
 	{
 	ODesc d;
 	d.SetShort();

testing/btest/plugins/hooks-plugin/src/Plugin.h

@@ -11,7 +11,7 @@ class Plugin : public ::plugin::Plugin
 {
 protected:
 	virtual int HookLoadFile(const std::string& file, const std::string& ext);
-	virtual Val* HookCallFunction(const Func* func, val_list* args);
+	virtual Val* HookCallFunction(const Func* func, Frame* frame, val_list* args);
 	virtual bool HookQueueEvent(Event* event);
 	virtual void HookDrainEvents();
 	virtual void HookUpdateNetworkTime(double network_time);