diff --git a/src/Func.cc b/src/Func.cc index 9a4ff6a4fb..547af2c6ce 100644 --- a/src/Func.cc +++ b/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 + char sbuf[1024]; + 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) { - /* - 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()); - reporter->InternalError(sbuf); - } + 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); - if ( plugin_result ) - return HandlePluginResult(plugin_result, args, Flavor()); + plugin_result = HandlePluginResult(plugin_result, args, Flavor()); + if(plugin_result) + { + 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); - if ( plugin_result ) - return HandlePluginResult(plugin_result, args, FUNC_FLAVOR_FUNCTION); + plugin_result = HandlePluginResult(plugin_result, args, FUNC_FLAVOR_FUNCTION); + if(plugin_result) + { + Val *result = plugin_result->value; + delete plugin_result; + return result; + } if ( g_trace_state.DoTrace() ) { diff --git a/src/Func.h b/src/Func.h index 446043d581..bfaa24708b 100644 --- a/src/Func.h +++ b/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); diff --git a/src/plugin/Manager.cc b/src/plugin/Manager.cc index 60e4d4fb78..d6ac3866ea 100644 --- a/src/plugin/Manager.cc +++ b/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; } diff --git a/src/plugin/Manager.h b/src/plugin/Manager.h index 02071fa5b7..75d15a5c8b 100644 --- a/src/plugin/Manager.h +++ b/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. diff --git a/src/plugin/Plugin.cc b/src/plugin/Plugin.cc index 9565236d81..8aa528bc3d 100644 --- a/src/plugin/Plugin.cc +++ b/src/plugin/Plugin.cc @@ -106,6 +106,21 @@ void HookArgument::Describe(ODesc* d) const d->Add(""); break; + case WRAPPED_VAL: + if ( arg.wrapper ) + { + d->Add("wrapped("); + if(arg.wrapper->value) + { + arg.wrapper->value->Describe(d); + } + else + d->Add(""); + 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; } diff --git a/src/plugin/Plugin.h b/src/plugin/Plugin.h index a921047b09..9119d338d5 100644 --- a/src/plugin/Plugin.h +++ b/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 diff --git a/testing/btest/plugins/hooks-plugin/src/Plugin.cc b/testing/btest/plugins/hooks-plugin/src/Plugin.cc index a9d0a529ba..e5507f5a0e 100644 --- a/testing/btest/plugins/hooks-plugin/src/Plugin.cc +++ b/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(); diff --git a/testing/btest/plugins/hooks-plugin/src/Plugin.h b/testing/btest/plugins/hooks-plugin/src/Plugin.h index 3bfa66a83f..940e427621 100644 --- a/testing/btest/plugins/hooks-plugin/src/Plugin.h +++ b/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);