Merge remote-tracking branch 'origin/topic/gilbert/plugin-api-tweak'

* origin/topic/gilbert/plugin-api-tweak:
  Updating plugin.hooks baseline so that test succeeds
  Revert spacing change that shouldn't have been included with the previous changeset ... should fix all of the plugin tests save hooks, which needs to be updated.
  More small fixes
  Small fixes
  Incremental
  Re-updating plugin.hooks test to include new argument output (after merge).
  Fixing logic errors in HandlePluginResult
  Updating tests and tweaking HookArgument to include Frame support.
  Incremental commit: implementing a wrapper for the Val class.
  Reverting change to const status of network_time.  Also, see FIXME: in Func.cc / HandlePluginResult ...
  Tweaks to result handling to make things a little more sane.
  Plugin API: minor change (adding parent frame) to support calling methods from hook.  Also declare network time update argument to be const because good practice.

BIT-1270 #merged

Conflicts:
	testing/btest/Baseline/plugins.hooks/output

CHANGES

@@ -1,4 +1,13 @@
 
+2.3-511 | 2015-03-02 18:07:17 -0800
+
+  * Changes to plugin meta hooks for function calls. (Gilbert Clark)
+
+        - Add frame argument.
+
+        - Change return value to tuple unambigiously whether hook
+          returned a result.
+
 2.3-493 | 2015-03-02 17:17:32 -0800
 
   * Extend the SSL weak-keys policy file to also alert when

VERSION

@@ -1 +1 @@
-2.3-493
+2.3-511

aux/btest

@@ -1 +1 @@
-Subproject commit 93d4989ed1537e4d143cf09d44077159f869a4b2
+Subproject commit d69df586c91531db0c3abe838b10a429dda4fa87

src/Func.cc

@@ -54,6 +54,7 @@ const Expr* calling_expr = 0;
 bool did_builtin_init = false;
 
 vector<Func*> Func::unique_ids;
+static const std::pair<bool, Val*> empty_hook_result(false, NULL);
 
 Func::Func() : scope(0), type(0)
 	{
@@ -245,20 +246,31 @@ TraversalCode Func::Traverse(TraversalCallback* cb) const
 	HANDLE_TC_STMT_POST(tc);
 	}
 
-Val* Func::HandlePluginResult(Val* plugin_result, val_list* args, function_flavor flavor) const
+std::pair<bool, Val*> Func::HandlePluginResult(std::pair<bool, Val*> plugin_result, val_list* args, function_flavor flavor) const
 	{
-	// Helper function factoring out this code from BroFunc:Call() for better
+	// Helper function factoring out this code from BroFunc:Call() for
-	// readability.
+	// better readability.
+
+	if( ! plugin_result.first )
+		{
+		if( plugin_result.second )
+			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;
+		}
 
 	switch ( flavor ) {
 	case FUNC_FLAVOR_EVENT:
-		Unref(plugin_result);
+		if( plugin_result.second )
-		plugin_result = 0;
+			reporter->InternalError("plugin returned non-void result for event %s", this->Name());
+
 		break;
 
 	case FUNC_FLAVOR_HOOK:
-		if ( plugin_result->Type()->Tag() != TYPE_BOOL )
+		if ( plugin_result.second->Type()->Tag() != TYPE_BOOL )
-			reporter->InternalError("plugin returned non-bool for hook");
+			reporter->InternalError("plugin returned non-bool for hook %s", this->Name());
 
 		break;
 
@@ -268,14 +280,14 @@ Val* Func::HandlePluginResult(Val* plugin_result, val_list* args, function_flavo
 
 		if ( (! yt) || yt->Tag() == TYPE_VOID )
 			{
-			Unref(plugin_result);
+			if( plugin_result.second )
-			plugin_result = 0;
+				reporter->InternalError("plugin returned non-void result for void method %s", this->Name());
 			}
 
-		else
+		else if ( plugin_result.second && plugin_result.second->Type()->Tag() != yt->Tag() && yt->Tag() != TYPE_ANY)
 			{
-			if ( plugin_result->Type()->Tag() != yt->Tag() )
+			reporter->InternalError("plugin returned wrong type (got %d, expecting %d) for %s",
-				reporter->InternalError("plugin returned wrong type for function call");
+						plugin_result.second->Type()->Tag(), yt->Tag(), this->Name());
 			}
 
 		break;
@@ -331,10 +343,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, args), 0);
+	std::pair<bool, Val*> plugin_result = PLUGIN_HOOK_WITH_RESULT(HOOK_CALL_FUNCTION, HookCallFunction(this, parent, args), empty_hook_result);
 
-	if ( plugin_result )
+	plugin_result = HandlePluginResult(plugin_result, args, Flavor());
-		return HandlePluginResult(plugin_result, args, Flavor());
+
+	if( plugin_result.first )
+		{
+		Val *result = plugin_result.second;
+		return result;
+		}
 
 	if ( bodies.empty() )
 		{
@@ -548,10 +565,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, args), 0);
+	std::pair<bool, Val*> plugin_result = PLUGIN_HOOK_WITH_RESULT(HOOK_CALL_FUNCTION, HookCallFunction(this, parent, args), empty_hook_result);
 
-	if ( plugin_result )
+	plugin_result = HandlePluginResult(plugin_result, args, FUNC_FLAVOR_FUNCTION);
-		return HandlePluginResult(plugin_result, args, FUNC_FLAVOR_FUNCTION);
+
+	if ( plugin_result.first )
+		{
+		Val *result = plugin_result.second;
+		return result;
+		}
 
 	if ( g_trace_state.DoTrace() )
 		{

src/Func.h

@@ -3,6 +3,8 @@
 #ifndef func_h
 #define func_h
 
+#include <utility>
+
 #include "BroList.h"
 #include "Obj.h"
 #include "Debug.h"
@@ -71,7 +73,7 @@ protected:
 	Func();
 
 	// Helper function for handling result of plugin hook.
-	Val* HandlePluginResult(Val* plugin_result, val_list* args, function_flavor flavor) const;
+	std::pair<bool, Val*> HandlePluginResult(std::pair<bool, Val*> plugin_result, val_list* args, function_flavor flavor) const;
 
 	DECLARE_ABSTRACT_SERIAL(Func);
 

src/plugin/Manager.cc

@@ -561,31 +561,34 @@ int Manager::HookLoadFile(const string& file)
 	return rc;
 	}
 
-Val* Manager::HookCallFunction(const Func* func, val_list* vargs) const
+std::pair<bool, Val*> Manager::HookCallFunction(const Func* func, Frame* parent, val_list* vargs) const
 	{
 	HookArgumentList args;
 
 	if ( HavePluginForHook(META_HOOK_PRE) )
 		{
 		args.push_back(HookArgument(func));
+		args.push_back(HookArgument(parent));
 		args.push_back(HookArgument(vargs));
 		MetaHookPre(HOOK_CALL_FUNCTION, args);
 		}
 
 	hook_list* l = hooks[HOOK_CALL_FUNCTION];
 
-	Val* v = 0;
+	std::pair<bool, Val*> v = std::pair<bool, Val*>(false, NULL);
 
 	if ( l )
+		{
 		for ( hook_list::iterator i = l->begin(); i != l->end(); ++i )
 			{
 			Plugin* p = (*i).second;
 
-			v = p->HookCallFunction(func, vargs);
+			v = p->HookCallFunction(func, parent, vargs);
 
-			if ( v )
+			if ( v.first )
 				break;
 			}
+		}
 
 	if ( HavePluginForHook(META_HOOK_POST) )
 		MetaHookPost(HOOK_CALL_FUNCTION, args, HookArgument(v));

src/plugin/Manager.h

@@ -3,6 +3,7 @@
 #ifndef PLUGIN_MANAGER_H
 #define PLUGIN_MANAGER_H
 
+#include <utility>
 #include <map>
 
 #include "Plugin.h"
@@ -244,7 +245,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, val_list* args) const;
+	std::pair<bool, Val*> HookCallFunction(const Func* func, Frame *parent, val_list* args) const;
 
 	/**
 	 * Hook that filters the queuing of an event.

src/plugin/Plugin.cc

@@ -83,6 +83,26 @@ void HookArgument::Describe(ODesc* d) const
 			d->Add("<null>");
 		break;
 
+	case FUNC_RESULT:
+		if ( func_result.first )
+			{
+			if( func_result.second )
+				func_result.second->Describe(d);
+			else
+				d->Add("<null>");
+			}
+		else
+			d->Add("<no result>");
+
+		break;
+
+	case FRAME:
+		if ( arg.frame )
+			d->Add("<frame>");
+		else
+			d->Add("<null>");
+		break;
+
 	case FUNC:
 		if ( arg.func )
 			d->Add(arg.func->Name());
@@ -271,9 +291,10 @@ int Plugin::HookLoadFile(const std::string& file, const std::string& ext)
 	return -1;
 	}
 
-Val* Plugin::HookCallFunction(const Func* func, val_list* args)
+std::pair<bool, Val*> Plugin::HookCallFunction(const Func* func, Frame *parent, val_list* args)
 	{
-	return 0;
+	std::pair<bool, Val*> result(false, NULL);
+	return result;
 	}
 
 bool Plugin::HookQueueEvent(Event* event)

src/plugin/Plugin.h

@@ -5,6 +5,7 @@
 
 #include <list>
 #include <string>
+#include <utility>
 
 #include "config.h"
 #include "analyzer/Component.h"
@@ -156,7 +157,7 @@ public:
 	 * Type of the argument.
 	 */
 	enum Type {
-		BOOL, DOUBLE, EVENT, FUNC, INT, STRING, VAL, VAL_LIST, VOID, VOIDP,
+		BOOL, DOUBLE, EVENT, FRAME, FUNC, FUNC_RESULT, INT, STRING, VAL, VAL_LIST, VOID, VOIDP
 	};
 
 	/**
@@ -209,6 +210,16 @@ public:
 	 */
 	HookArgument(void* p)	{ type = VOIDP; arg.voidp = p; }
 
+	/**
+	 * Constructor with a function result argument.
+	 */
+	HookArgument(std::pair<bool, Val*> fresult)	{ type = FUNC_RESULT; func_result = fresult; }
+
+	/**
+	 * Constructor with a Frame argument.
+	 */
+	HookArgument(Frame* f)	{ type = FRAME; arg.frame = f; }
+
 	/**
 	 * Returns the value for a boolen argument. The argument's type must
 	 * match accordingly.
@@ -251,6 +262,18 @@ 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 std::pair<bool, Val*> AsFuncResult() const { assert(type == FUNC_RESULT); return func_result; }
+
+	/**
+	 * Returns the value for a Bro frame argument.  The argument's type must
+	 * match accordingly.
+	 */
+	const Frame* AsFrame() const { assert(type == FRAME); return arg.frame; }
+
 	/**
 	 * Returns the value for a list of Bro values argument. The argument's type must
 	 * match accordingly.
@@ -282,13 +305,16 @@ private:
 		double double_;
 		const Event* event;
 		const Func* func;
+		const Frame* frame;
 		int int_;
 		const Val* val;
 		const val_list* vals;
 		const void* voidp;
 	} arg;
 
-	std::string arg_string; // Outside union because it has dtor.
+	// Outside union because these have dtors.
+	std::pair<bool, Val*> func_result;
+	std::string arg_string;
 };
 
 typedef std::list<HookArgument> HookArgumentList;
@@ -568,13 +594,13 @@ 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
+	 * @return If the plugin handled the call, a std::pair<bool, Val*> with the
-	 * count containixnmg the result value to pass back to the interpreter
+	 * processed flag set to true, and a value set on the object with
-	 * (for void functions and events any \a Val is fine; it will be
+	 * a+1 reference count containing the result value to pass back to the
-	 * ignored; best to use a \c TYPE_ANY). If the plugin did not handle
+	 * interpreter.  If the plugin did not handle the call, it must
-	 * the call, it must return null.
+	 * return a pair with the processed flag set to 'false'.
 	 */
-	virtual Val* HookCallFunction(const Func* func, val_list* args);
+	virtual std::pair<bool, Val*> HookCallFunction(const Func* func, Frame *parent, val_list* args);
 
 	/**
 	 * Hook into raising events. Whenever the script interpreter is about
@@ -606,7 +632,7 @@ protected:
 	 * Hook for updates to network time. This method will be called
 	 * whenever network time is advanced.
 	 *
-	 * @param networkt_time The new network time.
+	 * @param network_time The new network time.
 	 */
 	virtual void HookUpdateNetworkTime(double network_time);
 

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)
+std::pair<bool, Val*> Plugin::HookCallFunction(const Func* func, Frame* frame, val_list* args)
 	{
 	ODesc d;
 	d.SetShort();
@@ -57,7 +57,7 @@ Val* Plugin::HookCallFunction(const Func* func, val_list* args)
 	fprintf(stderr, "%.6f %-15s %s\n", network_time, "| HookCallFunction",
 		d.Description());
 
-	return 0;
+	return std::pair<bool, Val*>(false, NULL);
 	}
 
 bool Plugin::HookQueueEvent(Event* event)

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 std::pair<bool, Val*> HookCallFunction(const Func* func, Frame* frame, val_list* args);
 	virtual bool HookQueueEvent(Event* event);
 	virtual void HookDrainEvents();
 	virtual void HookUpdateNetworkTime(double network_time);