Polishing, mostly documentation updates.

doc/devel/plugins.rst

@@ -263,7 +263,6 @@ activated, and hence show up as such even in bare mode.
 
     Is this the right activation model?
 
-
 Plugin Component
 ================
 

src/broxygen/Target.cc

@@ -265,7 +265,7 @@ void ProtoAnalyzerTarget::DoCreateAnalyzerDoc(FILE* f) const
 
 	WriteAnalyzerTagDefn(f, "Analyzer");
 
-	plugin::Manager::plugin_list plugins = plugin_mgr->Plugins();
+	plugin::Manager::plugin_list plugins = plugin_mgr->ActivePlugins();
 	plugin::Manager::plugin_list::const_iterator it;
 
 	for ( it = plugins.begin(); it != plugins.end(); ++it )
@@ -293,7 +293,7 @@ void FileAnalyzerTarget::DoCreateAnalyzerDoc(FILE* f) const
 
 	WriteAnalyzerTagDefn(f, "Files");
 
-	plugin::Manager::plugin_list plugins = plugin_mgr->Plugins();
+	plugin::Manager::plugin_list plugins = plugin_mgr->ActivePlugins();
 	plugin::Manager::plugin_list::const_iterator it;
 
 	for ( it = plugins.begin(); it != plugins.end(); ++it )

src/main.cc

@@ -252,7 +252,7 @@ void usage()
 
 void show_plugins(int level)
 	{
-	plugin::Manager::plugin_list plugins = plugin_mgr->Plugins();
+	plugin::Manager::plugin_list plugins = plugin_mgr->ActivePlugins();
 
 	if ( ! plugins.size() )
 		{

src/plugin/Component.h

@@ -16,7 +16,7 @@ namespace component {
  */
 enum Type {
 	READER,	/// An input reader (not currently used).
-	WRITER,	/// An logging writer (not currenly used).
+	WRITER,	/// A logging writer (not currenly used).
 	ANALYZER,	/// A protocol analyzer.
 	FILE_ANALYZER	/// A file analyzer.
 	};
@@ -59,18 +59,19 @@ public:
 	 * Returns a textual representation of the component. This goes into
 	 * the output of "bro -NN".
 	 *
-	 * By default version, this just outputs the type and the name.
+	 * By default, this just outputs the type and the name. Derived
-	 * Derived versions should override DoDescribe() to add type specific
+	 * versions can override DoDescribe() to add type specific details.
-	 * details.
 	 *
 	 * @param d The description object to use.
 	 */
-	virtual void Describe(ODesc* d) const;
+	void Describe(ODesc* d) const;
 
 protected:
 	/**
 	 * Adds type specific information to the outout of Describe().
 	 *
+	 * The default version does nothing.
+	 *
 	 * @param d The description object to use.
 	  */
 	virtual void DoDescribe(ODesc* d) const	{ }

src/plugin/ComponentManager.h

@@ -117,7 +117,6 @@ public:
 	C* Lookup(EnumVal* val) const;
 
 private:
-
 	string module; /**< Script layer module in which component tags live. */
 	EnumType* tag_enum_type; /**< Enum type of component tags. */
 	map<string, C*> components_by_name;

src/plugin/Manager.cc

@@ -292,14 +292,14 @@ static bool plugin_cmp(const Plugin* a, const Plugin* b)
 
 void Manager::RegisterPlugin(Plugin *plugin)
 	{
-	Manager::PluginsInternal()->push_back(plugin);
+	Manager::ActivePluginsInternal()->push_back(plugin);
 
 	if ( current_dir.size() && current_sopath.size() )
 		// A dynamic plugin, record its location.
 		plugin->SetPluginLocation(current_dir.c_str(), current_sopath.c_str());
 
 	// Sort plugins by name to make sure we have a deterministic order.
-	PluginsInternal()->sort(plugin_cmp);
+	ActivePluginsInternal()->sort(plugin_cmp);
 
 	current_plugin = plugin;
 	}
@@ -320,7 +320,8 @@ void Manager::InitPreScript()
 	{
 	assert(! init);
 
-	for ( plugin_list::iterator i = Manager::PluginsInternal()->begin(); i != Manager::PluginsInternal()->end(); i++ )
+	for ( plugin_list::iterator i = Manager::ActivePluginsInternal()->begin();
+          i != Manager::ActivePluginsInternal()->end(); i++ )
 		{
 		Plugin* plugin = *i;
 		plugin->DoConfigure();
@@ -334,7 +335,8 @@ void Manager::InitBifs()
 	{
 	bif_init_func_map* bifs = BifFilesInternal();
 
-	for ( plugin_list::iterator i = Manager::PluginsInternal()->begin(); i != Manager::PluginsInternal()->end(); i++ )
+	for ( plugin_list::iterator i = Manager::ActivePluginsInternal()->begin();
+          i != Manager::ActivePluginsInternal()->end(); i++ )
 		{
 		bif_init_func_map::const_iterator b = bifs->find((*i)->Name());
 
@@ -350,7 +352,8 @@ void Manager::InitPostScript()
 	{
 	assert(init);
 
-	for ( plugin_list::iterator i = Manager::PluginsInternal()->begin(); i != Manager::PluginsInternal()->end(); i++ )
+	for ( plugin_list::iterator i = Manager::ActivePluginsInternal()->begin();
+          i != Manager::ActivePluginsInternal()->end(); i++ )
 		(*i)->InitPostScript();
 	}
 
@@ -358,25 +361,23 @@ void Manager::FinishPlugins()
 	{
 	assert(init);
 
-	for ( plugin_list::iterator i = Manager::PluginsInternal()->begin(); i != Manager::PluginsInternal()->end(); i++ )
+	for ( plugin_list::iterator i = Manager::ActivePluginsInternal()->begin();
-		{
+          i != Manager::ActivePluginsInternal()->end(); i++ )
 		(*i)->Done();
-//		delete *i;
-		}
 
-	Manager::PluginsInternal()->clear();
+	Manager::ActivePluginsInternal()->clear();
 
 	init = false;
 	}
 
-Manager::plugin_list Manager::Plugins() const
+Manager::plugin_list Manager::ActivePlugins() const
 	{
-	return *Manager::PluginsInternal();
+	return *Manager::ActivePluginsInternal();
 	}
 
 Manager::inactive_plugin_list Manager::InactivePlugins() const
 	{
-	plugin_list* all = PluginsInternal();
+	plugin_list* all = ActivePluginsInternal();
 
 	inactive_plugin_list inactives;
 
@@ -400,7 +401,7 @@ Manager::inactive_plugin_list Manager::InactivePlugins() const
 	return inactives;
 	}
 
-Manager::plugin_list* Manager::PluginsInternal()
+Manager::plugin_list* Manager::ActivePluginsInternal()
 	{
 	static plugin_list* plugins = 0;
 

src/plugin/Manager.h

@@ -12,14 +12,31 @@
 
 namespace plugin {
 
-// Macros that trigger a plugin hook. We put this into macros to short-cut
+// Macros that trigger plugin hooks. We put this into macros to short-cut the
-// the code for the most common case that no plugin defines the hook.
+// code for the most common case that no plugin defines the hook.
-#define PLUGIN_HOOK_WITH_RESULT(hook, method_call, default_result) \
-   (plugin_mgr->HavePluginForHook(::plugin::hook) ? plugin_mgr->method_call : (default_result))
 
+/**
+ * Macro to trigger hooks without result.
+ *
+ * @param hook The \a plugin::HookType constant corresponding to the hook to trigger.
+ *
+ * @param method_call The \a Manager method corresponding to the hook.
+ */
 #define PLUGIN_HOOK_VOID(hook, method_call) \
 	if ( plugin_mgr->HavePluginForHook(plugin::hook) ) plugin_mgr->method_call;
 
+/**
+ * Macro to trigger hooks that return a result.
+ *
+ * @param hook The \a plugin::HookType constant corresponding to the hook to trigger.
+ *
+ * @param method_call The \a Manager method corresponding to the hook.
+ *
+ * @param default_result: The result to use if there's no plugin implementing
+ * the hook.
+ */
+#define PLUGIN_HOOK_WITH_RESULT(hook, method_call, default_result) \
+	(plugin_mgr->HavePluginForHook(::plugin::hook) ? plugin_mgr->method_call : (default_result))
 
 /**
  * A singleton object managing all plugins.
@@ -43,24 +60,24 @@ public:
 	virtual ~Manager();
 
 	/**
-	 * Searches a set of directories for plugins. If a specificed
+	 * Searches a set of directories for plugins. If a specified directory
-	 * directory does not contain a plugin itself, the method searches
+	 * does not contain a plugin itself, the method searches for plugins
-	 * for plugins recursively. For plugins found, the method makes them
+	 * recursively. For plugins found, the method makes them available for
-	 * available for later activation via ActivatePlugin().
+	 * later activation via ActivatePlugin().
 	 *
 	 * This must be called only before InitPluginsPreScript().
 	 *
-	 * @param dir The directory to search for plugins. Multiple
+	 * @param dir The directory to search for plugins. Multiple directories
-	 * directories are split by ':'.
+	 * can be given by splitting them with ':'.
 	 */
 	void SearchDynamicPlugins(const std::string& dir);
 
 	/**
 	 * Activates a plugin that SearchPlugins() has previously discovered.
-	 * Activing a plugin involved loading its dynamic module, making its
+	 * Activating a plugin involves loading its dynamic module, making its
 	 * bifs available, and adding its script paths to BROPATH.
 	 *
-	 * @param name The name of the plugin, as determined previously by
+	 * @param name The name of the plugin, as found previously by
 	 * SearchPlugin().
 	 *
 	 * @return True if the plugin has been loaded successfully.
@@ -70,15 +87,15 @@ public:
 
 	/**
 	 * Activates plugins that SearchPlugins() has previously discovered. The
-	 * effect is the same all calling \a ActivePlugin(name) for the plugins.
+	 * effect is the same all calling \a ActivePlugin(name) for each plugin.
 	 *
 	 * @param all If true, activates all plugins that are found. If false,
-	 * activate only those that should always be activated unconditionally
+	 * activates only those that should always be activated unconditionally,
-	 * via the BRO_PLUGIN_ACTIVATE enviroment variable. In other words, it's
+	 * as specified via the BRO_PLUGIN_ACTIVATE enviroment variable. In other
-	 * false if running bare mode.
+	 * words, it's \c true in standard mode and \c false in bare mode.
 	 *
 	 * @return True if all plugins have been loaded successfully. If one
-	 * fail to load, the method stops there without loading any furthers
+	 * fails to load, the method stops there without loading any further ones
 	 * and returns false.
 	 */
 	bool ActivateDynamicPlugins(bool all);
@@ -91,30 +108,30 @@ public:
 	void InitPreScript();
 
 	/**
-	 * Second-stage initialization of the manager. This is called in
+	 * Second-stage initialization of the manager. This is called in between
-	 * between pre- and post-script to make BiFs available.
+	 * pre- and post-script to make BiFs available.
 	 */
 	void InitBifs();
 
 	/**
-	 * Third-stage initialization of the manager. This is called late
+	 * Third-stage initialization of the manager. This is called late during
-	 * during Bro's initialization after any scripts are processed, and
+	 * Bro's initialization after any scripts are processed, and forwards to
-	 * forwards to the corresponding Plugin methods.
+	 * the corresponding Plugin methods.
 	 */
 	void InitPostScript();
 
 	/**
 	 * Finalizes all plugins at termination time. This forwards to the
-	 * corresponding Plugin methods.
+	 * corresponding Plugin \a Done() methods.
 	 */
 	void FinishPlugins();
 
 	/**
-	 * Returns a list of all available and activated plugins. This includes
+	 * Returns a list of all available activated plugins. This includes all
-	 * all that are compiled in statically, as well as those loaded
+	 * that are compiled in statically, as well as those loaded dynamically
-	 * dynamically so far.
+	 * so far.
 	 */
-	plugin_list Plugins() const;
+	plugin_list ActivePlugins() const;
 
 	/**
 	 * Returns a list of all dynamic plugins that have been found, yet not
@@ -141,7 +158,7 @@ public:
 	 */
 	bool HavePluginForHook(HookType hook) const
 		{
-		// Inline to make avoid the function call.
+		// Inline to avoid the function call.
 		return hooks[hook] != 0;
 		}
 
@@ -174,8 +191,12 @@ public:
 	void DisableHook(HookType hook, Plugin* plugin);
 
 	/**
-	 * Register interest in an event. The event will then be raised, and
+	 * Registers interest in an event by a plugin, even if there's no handler
-	 * hence passed to the plugin, even if there no handler defined.
+	 * for it. Normally a plugin receives events through HookQueueEvent()
+	 * only if Bro actually has code to execute for it. By calling this
+	 * method, the plugin tells Bro to raise the event even if there's no
+	 * correspondong handler; it will then go into HookQueueEvent() just as
+	 * any other.
 	 *
 	 * @param handler The event being interested in.
 	 *
@@ -184,9 +205,9 @@ public:
 	void RequestEvent(EventHandlerPtr handler, Plugin* plugin);
 
 	/**
-	 * Register interest in the destruction of a BroObj instance. When
+	 * Register interest in the destruction of a BroObj instance. When Bro's
-	 * Bro's reference counting triggers the objects destructor to run,
+	 * reference counting triggers the objects destructor to run, the \a
-	 * the \a HookBroObjDtor will be called.
+	 * HookBroObjDtor will be called.
 	 *
 	 * @param handler The object being interested in.
 	 *
@@ -197,16 +218,16 @@ public:
 	// Hook entry functions.
 
 	/**
-	 * Hook that gives plugins a chance to take over loading an input
+	 * Hook that gives plugins a chance to take over loading an input input
-	 * input file. This method must be called between InitPreScript() and
+	 * file. This method must be called between InitPreScript() and
 	 * InitPostScript() for each input file Bro is about to load, either
-	 * given on the command line or via @load script directives. The hook
+	 * given on the command line or via @load script directives. The hook can
-	 * can take over the file, in which case Bro must not further process
+	 * take over the file, in which case Bro must not further process it
-	 * it otherwise.
+	 * otherwise.
 	 *
-	 * @return 1 if a plugin took over the file and loaded it
+	 * @return 1 if a plugin took over the file and loaded it successfully; 0
-	 * successfully; 0 if a plugin took over the file but had trouble
+	 * if a plugin took over the file but had trouble loading it; and -1 if
-	 * loading it; and -1 if no plugin was interested in the file at all.
+	 * no plugin was interested in the file at all.
 	 */
 	virtual int HookLoadFile(const string& file);
 
@@ -215,12 +236,13 @@ public:
 	 *
 	 * @param func The function to be called.
 	 *
-	 * @param args The function call's arguments; they may be modified.
+	 * @param args The function call's arguments; they may be modified by the
+	 * method.
 	 *
-	 * @return If a plugin handled the call, a +1 Val with the result
+	 * @return If a plugin handled the call, a Val with a +1 reference count
-	 * value to pass back to the interpreter (for void functions and
+	 * containing the result value to pass back to the interpreter (for void
-	 * events, it may be any Val and must be ignored). If no plugin
+	 * functions and events, it may be any Val and must be ignored). If no
-	 * handled the call, the method returns null.
+	 * plugin handled the call, the method returns null.
 	 */
 	Val* HookCallFunction(const Func* func, val_list* args) const;
 
@@ -263,7 +285,12 @@ public:
 	static void RegisterPlugin(Plugin* plugin);
 
 	/**
-	 * Internal method that registers a bif file's init function for a plugin.
+	 * Internal method that registers a bif file's init function for a
+	 * plugin.
+	 *
+	 * @param plugin The plugin to reguster the function for.
+	 *
+	 * @param c The init function to register.
 	 */
 	static void RegisterBifFile(const char* plugin, bif_init_func c);
 
@@ -277,12 +304,12 @@ private:
 	typedef std::map<std::string, std::string> dynamic_plugin_map;
 	dynamic_plugin_map dynamic_plugins;
 
-	// We buffer scripts to load temporarliy to get them to load in the
+	// We temporarliy buffer scripts to load to get them to load in the
 	// right order.
 	typedef std::list<std::string> file_list;
 	file_list scripts_to_load;
 
-	bool init;
+	bool init;	// Flag indicating whether InitPreScript() has run yet.
 
 	// A hook list keeps pairs of plugin and priority interested in a
 	// given hook.
@@ -292,6 +319,7 @@ private:
 	// of that type enabled.
 	hook_list** hooks;
 
+	// Helpers providing access to current state during dlopen().
 	static Plugin* current_plugin;
 	static string current_dir;
 	static string current_sopath;
@@ -299,7 +327,7 @@ private:
 	// Returns a modifiable list of all plugins, both static and dynamic.
 	// This is a static method so that plugins can register themselves
 	// even before the manager exists.
-	static plugin_list* PluginsInternal();
+	static plugin_list* ActivePluginsInternal();
 
 	typedef std::list<bif_init_func> bif_init_func_list;
 	typedef std::map<std::string, bif_init_func_list*> bif_init_func_map;
@@ -315,7 +343,7 @@ std::list<T *> Manager::Components() const
 	{
 	std::list<T *> result;
 
-	for ( plugin_list::const_iterator p = PluginsInternal()->begin(); p != PluginsInternal()->end(); p++ )
+	for ( plugin_list::const_iterator p = ActivePluginsInternal()->begin(); p != ActivePluginsInternal()->end(); p++ )
 		{
 		component_list components = (*p)->Components();
 

src/plugin/Plugin.cc

@@ -206,18 +206,7 @@ void Plugin::InitPostScript()
 
 Plugin::bif_item_list Plugin::BifItems() const
 	{
-	bif_item_list l1 = bif_items;
+    return bif_items;
-	bif_item_list l2 = CustomBifItems();
-
-	for ( bif_item_list::const_iterator i = l2.begin(); i != l2.end(); i++ )
-		l1.push_back(*i);
-
-	return l1;
-	}
-
-Plugin::bif_item_list Plugin::CustomBifItems() const
-	{
-	return bif_item_list();
 	}
 
 void Plugin::Done()

src/plugin/Plugin.h

@@ -10,7 +10,7 @@
 #include "analyzer/Component.h"
 #include "file_analysis/Component.h"
 
-#define BRO_PLUGIN_API_VERSION 2
+static const int BRO_PLUGIN_API_VERSION = 2;
 
 class ODesc;
 class Func;
@@ -28,16 +28,16 @@ class Plugin;
  */
 enum HookType {
 	// Note: when changing this table, update hook_name() in Plugin.cc.
-	HOOK_LOAD_FILE,
+	HOOK_LOAD_FILE,			//< Activates Plugin::HookLoadFile().
-	HOOK_CALL_FUNCTION,
+	HOOK_CALL_FUNCTION,		//< Activates Plugin::HookCallFunction().
-	HOOK_QUEUE_EVENT,
+	HOOK_QUEUE_EVENT,		//< Activates Plugin::HookQueueEvent().
-	HOOK_DRAIN_EVENTS,
+	HOOK_DRAIN_EVENTS,		//< Activates Plugin::HookDrainEvents()
-	HOOK_UPDATE_NETWORK_TIME,
+	HOOK_UPDATE_NETWORK_TIME,	//< Activates Plugin::HookUpdateNetworkTime.
-	HOOK_BRO_OBJ_DTOR,
+	HOOK_BRO_OBJ_DTOR,		//< Activates Plugin::HookBroObjDtor.
 
 	// Meta hooks.
-	META_HOOK_PRE,
+	META_HOOK_PRE,			//< Activates Plugin::MetaHookPre().
-	META_HOOK_POST,
+	META_HOOK_POST,			//< Activates Plugin::MetaHookPost().
 
 	// End marker.
 	NUM_HOOKS,
@@ -49,15 +49,20 @@ enum HookType {
 extern const char* hook_name(HookType h);
 
 /**
- * Helper class to capture a plugin's version. A boolean operator evaluates
+ * Helper class to capture a plugin's version.
- * to true if the version has been set.
+ * */
- */
 struct VersionNumber {
 	int major; //< Major version number;
 	int minor; //< Minor version number;
 
+	/**
+	 *  Constructor.
+	 */
 	VersionNumber()	{ major = minor = -1; }
 
+	/**
+	 *  Returns true if the version is set to a non-negative value.
+	 */
 	operator bool() const { return major >= 0 && minor >= 0; }
 };
 
@@ -133,32 +138,127 @@ private:
 class HookArgument
 {
 public:
+	/**
+	 * Type of the argument.
+	 */
 	enum Type {
 		BOOL, DOUBLE, EVENT, FUNC, INT, STRING, VAL, VAL_LIST, VOID, VOIDP,
 	};
 
+	/**
+	 * Default constructor initialized the argument with type VOID.
+	 */
 	HookArgument()	{ type = VOID; }
+
+	/**
+	 * Constructor with a boolean argument.
+	 */
 	HookArgument(bool a)	{ type = BOOL; arg.bool_ = a; }
+
+	/**
+	 * Constructor with a double argument.
+	 */
 	HookArgument(double a)	{ type = DOUBLE; arg.double_ = a; }
+
+	/**
+	 * Constructor with an event argument.
+	 */
 	HookArgument(const Event* a)	{ type = EVENT; arg.event = a; }
+
+	/**
+	 * Constructor with a function argument.
+	 */
 	HookArgument(const Func* a)	{ type = FUNC; arg.func = a; }
+
+	/**
+	 * Constructor with an integer  argument.
+	 */
 	HookArgument(int a)	{ type = INT; arg.int_ = a; }
+
+	/**
+	 * Constructor with a string argument.
+	 */
 	HookArgument(const std::string& a)	{ type = STRING; arg_string = a; }
+
+	/**
+	 * Constructor with a Bro value argument.
+	 */
 	HookArgument(const Val* a)	{ type = VAL; arg.val = a; }
+
+	/**
+	 * Constructor with a list of Bro values argument.
+	 */
 	HookArgument(const val_list* a)	{ type = VAL_LIST; arg.vals = a; }
+
+	/**
+	 * Constructor with a void pointer argument.
+	 */
 	HookArgument(void* p)	{ type = VOIDP; arg.voidp = p; }
 
+	/**
+	 * Returns the value for a boolen argument. The argument's type must
+	 * match accordingly.
+	 */
 	bool AsBool() const	{ assert(type == BOOL); return arg.bool_; }
+
+	/**
+	 * Returns the value for a double argument. The argument's type must
+	 * match accordingly.
+	 */
 	double AsDouble() const	{ assert(type == DOUBLE); return arg.double_; }
+
+	/**
+	 * Returns the value for an event argument. The argument's type must
+	 * match accordingly.
+	 */
 	const Event* AsEvent() const	{ assert(type == EVENT); return arg.event; }
+
+	/**
+	 * Returns the value for a function argument. The argument's type must
+	 * match accordingly.
+	 */
 	const Func* AsFunc() const	{ assert(type == FUNC); return arg.func; }
+
+	/**
+	 * Returns the value for an integer argument. The argument's type must
+	 * match accordingly.
+	 */
 	double AsInt() const	{ assert(type == INT); return arg.int_; }
+
+	/**
+	 * Returns the value for a string argument. The argument's type must
+	 * match accordingly.
+	 */
 	const std::string& AsString() const	{ assert(type == STRING); return arg_string; }
+
+	/**
+	 * Returns the value for a Bro value argument. The argument's type must
+	 * match accordingly.
+	 */
 	const Val* AsVal() const	{ assert(type == VAL); return arg.val; }
+
+	/**
+	 * Returns the value for a list of Bro values argument. The argument's type must
+	 * match accordingly.
+	 */
 	const val_list* AsValList() const	{ assert(type == VAL_LIST); return arg.vals; }
+
+	/**
+	 * Returns the value for a vod pointer argument. The argument's type
+	 * must match accordingly.
+	 */
 	const void* AsVoidPtr() const	{ assert(type == VOIDP); return arg.voidp; }
 
+	/**
+	 * Returns the argument's type.
+	 */
 	Type GetType() const	{ return type; }
+
+	/**
+	 * Returns a textual representation of the argument.
+	 *
+	 * @param d Description object to use for rendering.
+	 */
 	void Describe(ODesc* d) const;
 
 private:
@@ -185,24 +285,25 @@ typedef std::list<HookArgument> HookArgumentList;
  * Plugins encapsulate functionality that extends one or more of Bro's major
  * subsystems, such as analysis of a specific protocol, or logging output in
  * a particular format. A plugin acts a logical container that can provide a
- * set of different functionality. Specifically, it may:
+ * set of functionality. Specifically, it may:
  *
  * - Provide one or more \a components implementing functionality. For
  *   example, a RPC plugin could provide analyzer for set of related
  *   protocols (RPC, NFS, etc.), each of which would be a separate component.
- *   Likewise, a SQLite plugin could provide both a writer and reader
+ *   Likewise, an SQLite plugin could provide both a writer and reader
- *   component. In addition to components, a plugin can also provide of
+ *   component.
- *   script-level elements defined in *.bif files.
  *
- * - Provide BiF elements (functions, events, types, globals).
+ * - Provide BiF elements (functions, events, types, globals). Typically
+ *   they'll be defined in *.bif files, but a plugin can also create them
+ *   internally.
  *
  * - Provide hooks (aka callbacks) into Bro's core processing to inject
  *   and/or alter functionality.
  *
- * Note that a plugin needs to explicitly register all the functionality it
+ * A plugin needs to explicitly register all the functionality it provides.
- * provides. For components, it needs to call AddComponent(); for BiFs
+ * For components, it needs to call AddComponent(); for BiFs AddBifItem();
- * AddBifItem(); and for hooks EnableHook() and then also implemennt the
+ * and for hooks EnableHook() and then also implemennt the corresponding
- * corresponding virtual methods).
+ * virtual methods.
  *
  */
 class Plugin {
@@ -232,14 +333,14 @@ public:
 	const std::string& Description() const;
 
 	/**
-	 * Returns the version of the plugin. Version are only meaningful for
+	 * Returns the version of the plugin. Versions are only meaningful
-	 * dynamically compiled plugins; for statically compiled ones, this
+	 * for dynamically compiled plugins; for statically compiled ones,
-	 * will always return 0.
+	 * this will always return 0.
 	 */
 	VersionNumber Version() const;
 
 	/**
-	 * Returns true if this is a dynamically linked in plugin.
+	 * Returns true if this is a dynamically linked plugin.
 	 */
 	bool DynamicPlugin() const;
 
@@ -251,16 +352,17 @@ public:
 
 	/**
 	 * For dynamic plugins, returns the full path to the shared library
-	 * from which it was loaded. For static plugins, returns null.
+	 * from which it was loaded. For static plugins, returns an empty
+	 * string.
 	 **/
 	const std::string& PluginPath() const;
 
 	/**
-	 * Returns the internal API version that this plugin relies on. Only
+	 * Returns the internal version of the Bro API that this plugin
-	 * plugins that match Bro's current API version may be used. For
+	 * relies on. Only plugins that match Bro's current API version can
-	 * statically compiled plugins this is automatically the case, but
+	 * be used. For statically compiled plugins this is automatically the
-	 * dynamically loaded plugins may cause a mismatch if they were
+	 * case, but dynamically loaded plugins may cause a mismatch if they
-	 * compiled for a different Bro version.
+	 * were compiled for a different Bro version.
 	 */
 	int APIVersion() const;
 
@@ -284,7 +386,7 @@ public:
 	 *
 	 * @return A configuration describing the plugin.
 	 */
-	virtual Configuration Configure() { return Configuration(); } // TODO: Change to abstract method.
+	virtual Configuration Configure() = 0;
 
 	/**
 	 * First-stage initialization of the plugin called early during Bro's
@@ -325,7 +427,12 @@ public:
 	 * Another way to add this information is via overriding
 	 * CustomBifItems().
 	 *
-	 * \todo Do we need both this an CustomBifItems()?
+	 * Note that this method is rarely the right one to use. As it's for
+	 * informational purposes only, the plugin still needs to register
+	 * the BiF items themselves with the corresponding Bro parts. Doing
+	 * so can be tricky, and it's recommned to instead define BiF items
+	 * in separate *.bif files that the plugin then pulls in. If defined
+	 * there, one does *not* need to call this method.
 	 *
 	 * @param name The name of the BiF item.
 	 *
@@ -343,7 +450,8 @@ public:
 	 *
 	 * This method must not be called after InitPostScript().
 	 *
-	 * @param file The file to load. It will be searched along the standard paths.
+	 * @param file The file to load. It will be searched along the
+	 * standard paths.
 	 *
 	 * @return True if successful (which however may only mean
 	 * "successfully queued").
@@ -353,12 +461,6 @@ public:
 protected:
 	friend class Manager;
 
-	/**
-	 * Intializes the plugin's configutation. Called by the manager
-	 * before anything else.
-	 */
-	void DoConfigure();
-
 	/**
 	 * Registers and activates a component.
 	 *
@@ -372,12 +474,11 @@ protected:
 	 * have performance impaxct as many trigger frequently inside Bro's
 	 * main processing path.
 	 *
-	 * Note that hooks may be enabled/disabled dynamically at any time,
+	 * Note that while hooks may be enabled/disabled dynamically at any
-	 * the output of Bro's \c -NN option will only reflect that state at
+	 * time, the output of Bro's \c -NN option will only reflect their
-	 * startup time; hence usually one should call this for a plugin's
+	 * state at startup time. Usually one should call this method for a
-	 * hooks in either the plugin's ctor or in InitPreScript(). For
+	 * plugin's hooks in either the plugin's constructor or in
-	 * consistency with other parts of the API, there's a macro
+	 * InitPreScript().
-	 * PLUGIN_ENABLE_HOOK for use inside the ctor.
 	 *
 	 * @param hook The hook to enable.
 	 *
@@ -403,34 +504,27 @@ protected:
 	hook_list EnabledHooks() const;
 
 	/**
-	 * Register interest in an event. The event will then be raised, and
+	 * Registers interest in an event, even if there's no handler for it.
-	 * hence passed to the plugin, even if there no handler defined.
+	 * Normally a plugin receives events through HookQueueEvent() only if Bro
+	 * actually has code to execute for it. By calling this method, the
+	 * plugin tells Bro to raise the event even if there's no correspondong
+	 * handler; it will then go into HookQueueEvent() just as any other.
      *
-	 * @param handler The object being interested in.
+	 * @param handler The event handler being interested in.
 	 */
 	void RequestEvent(EventHandlerPtr handler);
 
 	/**
-	 * Register interest in the destruction of a BroObj instance. When
+	 * Registers interest in the destruction of a BroObj instance. When
 	 * Bro's reference counting triggers the objects destructor to run,
-	 * the \a HookBroObjDtor will be called.
+	 * \a HookBroObjDtor will be called.
+	 *
+	 * Note that his can get expensive if triggered for many objects.
 	 *
 	 * @param handler The object being interested in.
 	 */
 	void RequestBroObjDtor(BroObj* obj);
 
-	/**
-	 * Virtual method that can be overriden by derived class to provide
-	 * information about further script-level elements that the plugin
-	 * provides on its own, i.e., outside of the standard mechanism
-	 * processing *.bif files automatically. The returned information is
-	 * for informational purposes only and will show up in the result of
-	 * BifItems() as well as in the Describe() output.
-	 *
-	 * \todo Do we need both this an AddBifItem()?
-	 */
-	virtual bif_item_list CustomBifItems() const;
-
 	// Hook functions.
 
 	/**
@@ -439,18 +533,18 @@ protected:
 	 * timing guaranteed. It will be called once for each input file Bro
 	 * is about to load, either given on the command line or via @load
 	 * script directives. The hook can take over the file, in which case
-	 * Bro not further process it otherwise.
+	 * Bro will not further process it otherwise.
 	 *
-     * @param file The filename to be loaded.
+	 * @param file The filename to be loaded, including extension.
 	 *
-     * @param ext The extension of the filename. This is provided separately
+	 * @param ext The extension of the filename. This is provided
-     * just for convenience. The dot is excluded.
+	 * separately just for convenience. The dot is excluded.
 	 *
 	 * @return 1 if the plugin took over the file and loaded it
 	 * successfully; 0 if the plugin took over the file but had trouble
-	 * loading it (Bro will abort in this case, the plugin should have
+	 * loading it (Bro will abort in this case, and the plugin should
-	 * printed an error message); and -1 if the plugin wasn't interested
+	 * have printed an error message); and -1 if the plugin wasn't
-	 * in the file at all.
+	 * interested in the file at all.
 	 */
 	virtual int HookLoadFile(const std::string& file, const std::string& ext);
 
@@ -471,11 +565,11 @@ protected:
 	 * in place long as it ensures matching types and correct reference
 	 * counting.
 	 *
-	 * @return If the plugin handled the call, a +1 Val with the result
+	 * @return If the plugin handled the call, a Val with +1 reference
-	 * value to pass back to the interpreter (for void functions and
+	 * count containomg the result value to pass back to the interpreter
-	 * events any \a Val is fine; it will be ignored; best to use a \c
+	 * (for void functions and events any \a Val is fine; it will be
-	 * TYPE_ANY). If the plugin did not handle the call, it must return
+	 * ignored; best to use a \c TYPE_ANY). If the plugin did not handle
-	 * null.
+	 * the call, it must return null.
 	 */
 	virtual Val* HookCallFunction(const Func* func, val_list* args);
 
@@ -485,7 +579,7 @@ protected:
 	 * with this hook enabled a chance to handle the queuing otherwise
 	 * (in the order of their priorities). A plugin can either just
 	 * inspect the event, or take it over (i.e., prevent the interpreter
-	 * from queuing it it).
+	 * from queuing it itself).
 	 *
 	 * The default implementation does never handle the queuing in any
 	 * way.
@@ -501,7 +595,7 @@ protected:
 	virtual bool HookQueueEvent(Event* event);
 
 	/**
-	 * Hook intp event queue draining. This method will be called
+	 * Hook into event queue draining. This method will be called
 	 * whenever the event manager is draining its queue.
 	 */
 	virtual void HookDrainEvents();
@@ -529,7 +623,9 @@ protected:
 	// Meta hooks.
 
 	/**
-	 * A meta hook called just before another hook gets to execute.
+	 * A meta hook called just before another hook gets to execute. This
+	 * will be called independent of whether there's an implementation
+	 * for the hook.
 	 *
 	 * hook: The name of the hook about the execute. This will be the
 	 * same as the corresponding method name (e.g., \c HookQueueEvent).
@@ -541,7 +637,7 @@ protected:
 	virtual void MetaHookPre(HookType hook, const HookArgumentList& args);
 
 	/**
-	 * A meta hook called just after another hook gets to execute. This
+	 * A meta hook called just after another hook got to execute. This
 	 * will be called independent of whether there's an implementation
 	 * for the hook.
 	 *
@@ -555,14 +651,18 @@ protected:
 	 */
 	virtual void MetaHookPost(HookType hook, const HookArgumentList& args, HookArgument result);
 
-	// Methods that are used internally primarily.
+private:
+	/**
+	 * Intializes the plugin's internal configuration. Called by the
+	 * manager before anything else.
+	 */
+	void DoConfigure();
 
 	/**
 	 * Sets the base directory and shared library path from which the
 	 * plugin was loaded.
 	 *
-	 * This is used primarily internally; plugins will have there
+	 * This is called by the manager.
-	 * location set automatically.
 	 *
 	 * @param dir The plugin directory. The functions makes an internal
 	 * copy of string.
@@ -575,22 +675,20 @@ protected:
 	/**
 	 * Marks the plugin as dynamically loaded.
 	 *
-	 * This is used primarily internally; plugins will have this called
+	 * This is called by the manager.
-	 * by the manager.
 	 *
 	 * @param is_dynamic True if it's a dynamically loaded module.
 	 */
 	void SetDynamic(bool is_dynamic);
 
-private:
 	Configuration config;
 
 	std::string base_dir;	// The plugin's base directory.
 	std::string sopath;	// For dynamic plugins, the full path to the shared library.
 	bool dynamic;	// True if a dynamic plugin.
 
-	component_list components;
+	component_list components;	// Components the plugin provides.
-	bif_item_list bif_items;
+	bif_item_list bif_items;	// BiF items the plugin provides.
 };
 
 }