[1292_2] address review comments

src/lib/datasrc/datasrc_config.h.pre.in

@@ -17,13 +17,13 @@
 namespace isc {
 namespace datasrc {
 
-/// \brief Default directory to find the loadable data source modules
+/// \brief Default directory to find the loadable data source libraries
 ///
-/// This is the directory where, once installed, modules such as
-/// memory_ds.so and sqlite3_ds.so are found. It is used by the
+/// This is the directory where, once installed, loadable backend libraries
+/// such as memory_ds.so and sqlite3_ds.so are found. It is used by the
 /// DataSourceClient loader if no absolute path is used and
 /// B10_FROM_BUILD is not set in the environment.
-const char* const MODULE_PATH = "@@PKGLIBEXECDIR@@/";
+const char* const BACKEND_LIBRARY_PATH = "@@PKGLIBEXECDIR@@/";
 
 } // end namespace datasrc
 } // end namespace isc

src/lib/datasrc/factory.cc

@@ -44,22 +44,22 @@ getDataSourceLibFile(const std::string& type) {
     }
 
     // Type can be either a short name, in which case we need to
-    // append "_ds.so", or it can be a direct .so module.
+    // append "_ds.so", or it can be a direct .so library.
     std::string lib_file = type;
     const int ext_pos = lib_file.rfind(".so");
     if (ext_pos == std::string::npos || ext_pos + 3 != lib_file.length()) {
         lib_file.append("_ds.so");
     }
     // And if it is not an absolute path, prepend it with our
-    // module path
+    // loadable backend library path
     if (type[0] != '/') {
         // When running from the build tree, we do NOT want
-        // to load the installed module
+        // to load the installed loadable library
         if (getenv("B10_FROM_BUILD") != NULL) {
             lib_file = std::string(getenv("B10_FROM_BUILD")) +
                        "/src/lib/datasrc/.libs/" + lib_file;
         } else {
-            lib_file = isc::datasrc::MODULE_PATH + lib_file;
+            lib_file = isc::datasrc::BACKEND_LIBRARY_PATH + lib_file;
         }
     }
     return (lib_file);
@@ -74,7 +74,10 @@ LibraryContainer::LibraryContainer(const std::string& name) {
     // are recognized as such
     ds_lib_ = dlopen(name.c_str(), RTLD_NOW | RTLD_GLOBAL);
     if (ds_lib_ == NULL) {
-        isc_throw(DataSourceLibraryError, dlerror());
+        // This may cause the filename to appear twice in the actual
+        // error, but the output of dlerror is implementation-dependent
+        isc_throw(DataSourceLibraryError, "dlopen failed for " << name << 
+                                          ": " << dlerror());
     }
 }
 

src/lib/datasrc/factory.h

@@ -119,7 +119,9 @@ private:
 /// with '.so', the loader will not add '_ds.so'.
 /// It may also be an absolute path; if it starts with '/', nothing is
 /// prepended. If it does not, the loadable library will be taken from the
-/// libexec/backends/ installation directory.
+/// installation directory, see the value of
+/// isc::datasrc::BACKEND_LIBRARY_PATH in datasrc_config.h for the exact path.
+///
 /// \note When 'B10_FROM_BUILD' is set in the environment, the build
 ///       directory is used instead of the install directory.
 ///

src/lib/datasrc/tests/factory_unittest.cc

@@ -30,41 +30,41 @@ std::string SQLITE_DBFILE_EXAMPLE_ORG = TEST_DATA_DIR "/example.org.sqlite3";
 
 namespace {
 
+// note this helper only checks the error that is received up to the length
+// of the expected string. It will always pass if you give it an empty
+// expected_error
 void
-pathtest_helper(const std::string& file, const std::string& expected_error) {
+pathtestHelper(const std::string& file, const std::string& expected_error) {
     std::string error;
     try {
         DataSourceClientContainer(file, ElementPtr());
     } catch (const DataSourceLibraryError& dsle) {
         error = dsle.what();
     }
-    EXPECT_EQ(expected_error, error);
+    EXPECT_EQ(expected_error, error.substr(0, expected_error.size()));
 }
 
 TEST(FactoryTest, paths) {
     // Test whether the paths are made absolute if they are not,
     // by inspecting the error that is raised when they are wrong
-    const std::string error(": cannot open shared object file: "
-                            "No such file or directory");
+    const std::string error("dlopen failed for ");
     // With the current implementation, we can safely assume this has
     // been set for this test (as the loader would otherwise also fail
-    // unless the module happens to be installed)
+    // unless the loadable backend library happens to be installed)
     const std::string builddir(getenv("B10_FROM_BUILD"));
 
     // Absolute and ending with .so should have no change
-    pathtest_helper("/no_such_file.so", "/no_such_file.so" + error);
+    pathtestHelper("/no_such_file.so", error + "/no_such_file.so");
 
     // If no ending in .so, it should get _ds.so
-    pathtest_helper("/no_such_file", "/no_such_file_ds.so" + error);
+    pathtestHelper("/no_such_file", error + "/no_such_file_ds.so");
 
     // If not starting with /, path should be added. For this test that
     // means the build directory as set in B10_FROM_BUILD
-    pathtest_helper("no_such_file.so",
-                    builddir + "/src/lib/datasrc/.libs/no_such_file.so" +
-                    error);
-    pathtest_helper("no_such_file",
-                    builddir + "/src/lib/datasrc/.libs/no_such_file_ds.so" + 
-                    error);
+    pathtestHelper("no_such_file.so", error + builddir +
+                   "/src/lib/datasrc/.libs/no_such_file.so");
+    pathtestHelper("no_such_file", error + builddir +
+                   "/src/lib/datasrc/.libs/no_such_file_ds.so");
 }
 
 TEST(FactoryTest, sqlite3ClientBadConfig) {