[4297] Addressed review comments:

 - returnining different values in callout_params_library.cc
 - returned values are now described for getParameter
 - corrected index syntax comment in getParameter
 - added missing include
 - hooks_user.dox updated
 - hooks unit test moved, so it is now next to other hooks tests

6 files changed, 31 insertions, 28 deletions

diff --git a/src/lib/hooks/hooks_manager.h b/src/lib/hooks/hooks_manager.h
index 713fdfdbca..b6080be8eb 100644
--- a/src/lib/hooks/hooks_manager.h
+++ b/src/lib/hooks/hooks_manager.h
@@ -173,7 +173,7 @@ public:
 
     /// @brief Return list of loaded libraries with its parameters.
     ///
-    /// Returns the names of the loaded libraries and its parameters.
+    /// Returns the names of the loaded libraries and their parameters.
     ///
     /// @return List of loaded libraries (names + parameters)
     static HookLibsCollection getLibraryInfo();
diff --git a/src/lib/hooks/hooks_user.dox b/src/lib/hooks/hooks_user.dox
index 2c59aeddff..08d261d643 100644
--- a/src/lib/hooks/hooks_user.dox
+++ b/src/lib/hooks/hooks_user.dox
@@ -168,7 +168,7 @@ to namespaces.
 As the names suggest, "load" is called when a library is loaded and
 "unload" called when it is unloaded.  (It is always guaranteed that
 "load" is called: "unload" may not be called in some circumstances,
-e.g. if the system shuts down abnormally.)  These functions are the
+e.g., if the system shuts down abnormally.)  These functions are the
 places where any library-wide resources are allocated and deallocated.
 "load" is also the place where any callouts with non-standard names
 (names that are not hook point names) can be registered:
@@ -344,7 +344,7 @@ an argument as an @c int and the callout attempted to retrieve it as a
 @c long, an exception would be thrown even though any value that can
 be stored in an @c int will fit into a @c long.  This restriction also
 applies the "const" attribute but only as applied to data pointed to by
-pointers, e.g. if an argument is defined as a @c char*, an exception will
+pointers, e.g., if an argument is defined as a @c char*, an exception will
 be thrown if an attempt is made to retrieve it into a variable of type
 @c const @c char*.  (However, if an argument is set as a @c const @c int,
 it can be retrieved into an @c int.)  The documentation of each hook
@@ -434,7 +434,7 @@ status (see @ref hooksdgNextStep) was provided by
 a boolean flag called "Skip". However, since it only allowed to either continue
 or skip the next processing step and was not extensible to other decisions,
 setSkip(bool) call was replaced with a setStatus(enum) in Kea 1.0. This
-new approach is extensible. If we decide to add new results (e.g. WAIT
+new approach is extensible. If we decide to add new results (e.g., WAIT
 or RATELIMIT), we will be able to do so without changing the API again.
 
 If you have your hooks libraries that take advantage of skip flag, migrating
@@ -473,7 +473,7 @@ if (status == CalloutHandle::NEXT_STEP_SKIP) {
 @subsubsection hooksdgCalloutContext Per-Request Context
 
 Although the Kea modules can be characterized as handling a single
-packet at a time - e.g. the DHCPv4 server receives a DHCPDISCOVER packet,
+packet at a time - e.g., the DHCPv4 server receives a DHCPDISCOVER packet,
 processes it and responds with an DHCPOFFER, this may not always be true.
 Future developments may have the server processing multiple packets
 simultaneously, or to suspend processing on a packet and resume it at
@@ -962,7 +962,7 @@ only shared pointers have the correct behavior for the copy operation.)
 As briefly mentioned in @ref hooksdgExampleCallouts, the standard is for
 callouts in the user library to have the same name as the name of the
 hook to which they are being attached.  This convention was followed
-in the tutorial, e.g.  the callout that needed to be attached to the
+in the tutorial, e.g.,  the callout that needed to be attached to the
 "pkt4_receive" hook was named pkt4_receive.
 
 The reason for this convention is that when the library is loaded, the
@@ -1026,7 +1026,7 @@ a hook point.  Although it is likely to be rare for user code to need to
 do this, there may be instances where it make sense.
 
 To register multiple callouts on a hook, just call
-@c LibraryHandle::registerCallout multiple times on the same hook, e.g.
+@c LibraryHandle::registerCallout multiple times on the same hook, e.g.,
 
 @code
     libhandle.registerCallout("pkt4_receive", classify);
@@ -1184,7 +1184,7 @@ may be present, but will not affect the context values set by a library's
 callouts.
 
 Configuring multiple libraries just requires listing the libraries
-as separate elements of the hooks-libraries configuration element, e.g.
+as separate elements of the hooks-libraries configuration element, e.g.,
 
 @code
 "Dhcp4": {
@@ -1311,14 +1311,14 @@ specify it as an empty map. The third library is more interesting. It has five
 parameters specified. The first one called 'mail' is a string. The second one
 is an integer and the third one is boolean. Fourth and fifth parameters are
 slightly more complicated as they are a list and a map respectively. JSON
-structures can be nested if necessary, e.g. you can have a list, which contains
+structures can be nested if necessary, e.g., you can have a list, which contains
 maps, maps that contain maps that contain other maps etc. Any valid JSON
-structure can be repesented. One important limitation here is that the top
+structure can be represented. One important limitation here is that the top
 level "parameters" structure must either be a map or not present at all.
 
 Those parameters can be accessed in load() method. Passed isc::hooks::LibraryHandle
 object has a method called getParameter that returns an instance of
-isc::data::ConstElementPtr or NULL if there was no parameter specified. This pointer
+isc::data::ConstElementPtr or null pointer if there was no parameter specified. This pointer
 will point to an object derived from isc::data::Element class. For detailed
 explanation how to use those objects, see isc::data::Element class.
 
@@ -1338,12 +1338,12 @@ Here's a brief overview of how to use those elements:
    x->find("klingon"), x->contains("french"), x->size()
 
 Keep in mind that the user can structure his config file incorrectly.
-Remember to check if the structure has expected type before using type specific
+Remember to check if the structure has the expected type before using type specific
 method. For example calling stringValue on IntElement will throw an exception.
 You can do this by calling isc::data::Element::getType.
 
 Here's an example that obtains all of the parameters specified above.
-If you want to get nested elemented, Element::get(index) and Element::find(name)
+If you want to get nested elements, Element::get(index) and Element::find(name)
 will return ElementPtr, which can be iterated in similar manner.
 
 @code
@@ -1367,7 +1367,7 @@ int load(LibraryHandle& handle) {
 
     // In the following examples safety checks are omitted for clarity.
     // Make sure you do it properly similar to mail example above
-    // or you risk dereferencing NULL pointer or at least throwing
+    // or you risk dereferencing null pointer or at least throwing
     // an exception!
 
     // Integer handling example
diff --git a/src/lib/hooks/libinfo.h b/src/lib/hooks/libinfo.h
index 399d9148fe..42b0441586 100644
--- a/src/lib/hooks/libinfo.h
+++ b/src/lib/hooks/libinfo.h
@@ -13,6 +13,7 @@
 
 #include <string>
 #include <vector>
+#include <utility>
 
 namespace isc {
 namespace hooks {
diff --git a/src/lib/hooks/library_handle.cc b/src/lib/hooks/library_handle.cc
index 502e5ad98e..bdd42fde8f 100644
--- a/src/lib/hooks/library_handle.cc
+++ b/src/lib/hooks/library_handle.cc
@@ -85,17 +85,17 @@ LibraryHandle::getParameter(const std::string& name) {
     }
 
     // Some indexes have special meaning:
-    // - 0 - pre-user library callout
-    // - 1..numlib - indexes for actual libraries
-    // INT_MAX post-user library callout
+    // * 0 - pre-user library callout
+    // * 1..numlib - indexes for actual libraries
+    // * INT_MAX - post-user library callout
 
-    // Try to find appropriate parameter. May return NULL
+    // Try to find appropriate parameter. May return null pointer
     isc::data::ConstElementPtr params = libinfo[index - 1].second;
     if (!params) {
         return (isc::data::ConstElementPtr());
     }
 
-    // May return NULL if there's no parameter.
+    // May return null pointer if there's no parameter.
     return (params->get(name));
 }
 
diff --git a/src/lib/hooks/library_handle.h b/src/lib/hooks/library_handle.h
index a64178624c..bc695d7716 100644
--- a/src/lib/hooks/library_handle.h
+++ b/src/lib/hooks/library_handle.h
@@ -149,7 +149,7 @@ public:
     /// - x = getParameter("users") will return an instance of ListElement.
     ///   Its content can be accessed with the following methods:
     ///   x->size(), x->get(index)
-    /// - x = getParameter("watch-list") will return an instance of isc::data::MapElement.
+    /// - x = getParameter("header") will return an instance of isc::data::MapElement.
     ///   Its content can be accessed with the following methods:
     ///   x->find("klingon"), x->contains("french"), x->size()
     ///
@@ -166,6 +166,8 @@ public:
     /// unittests in data_unittests.cc.
     ///
     /// @param name text name of the parameter.
+    /// @return ElementPtr representing requested parameter (may be null, if
+    ///         there is no such parameter.)
     isc::data::ConstElementPtr
     getParameter(const std::string& name);
 
diff --git a/src/lib/hooks/tests/callout_params_library.cc b/src/lib/hooks/tests/callout_params_library.cc
index d934f5b1fa..b5b569a206 100644
--- a/src/lib/hooks/tests/callout_params_library.cc
+++ b/src/lib/hooks/tests/callout_params_library.cc
@@ -48,7 +48,7 @@ int load(LibraryHandle& handle) {
 
     if (string_elem->getType() != Element::string) {
         // Parameter is specified, but it's not a string.
-        return (1);
+        return (2);
     }
 
     std::string str = string_elem->stringValue();
@@ -58,43 +58,43 @@ int load(LibraryHandle& handle) {
         // This library is used for testing, so it expects exact value of the
         // parameter. Normal library would likely use whatever value user
         // specified.
-        return (1);
+        return (3);
     }
 
     // Integer handling example
     if (!int_elem) {
         // Parameter was not specified at all.
-        return (1);
+        return (4);
     }
 
     if (int_elem->getType() != Element::integer) {
         // Parameter is specified, but it's not an integer.
-        return (1);
+        return (5);
     }
 
     int int_value = int_elem->intValue();
     if (int_value != 42) {
         // Parameter specified, is an integer, but has a value different than
         // expected.
-        return (1);
+        return (6);
     }
 
     // Boolean handling example
     if (!bool_elem) {
         // Parameter was not specified at all.
-        return (1);
+        return (7);
     }
 
     if (bool_elem->getType() != Element::boolean) {
         // Parameter is specified, but it's not a boolean.
-        return (1);
+        return (8);
     }
 
     bool flag = bool_elem->boolValue();
     if (flag != true) {
         // Parameter specified, is a boolean, but has a value different than
         // expected.
-        return (1);
+        return (9);
     }
 
     // All validation steps were successful. The library has all the parameters