cleaning up moduleloader - incomplete

include/error.hpp

@@ -7,6 +7,10 @@ enum class DPMError {
     PATH_NOT_DIRECTORY,
     PERMISSION_DENIED,
     MODULE_NOT_FOUND,
+    MODULE_NOT_LOADED,
     MODULE_LOAD_FAILED,
-    INVALID_MODULE
+    INVALID_MODULE,
+    SYMBOL_NOT_FOUND,
+    SYMBOL_EXECUTION_FAILED,
+    UNDEFINED_ERROR
 };

src/ModuleLoader.cpp

@@ -52,96 +52,174 @@ DPMError ModuleLoader::load_module(const std::string& module_name, void*& module
         return DPMError::MODULE_LOAD_FAILED;
     }
 
-    dlerror();
+    std::vector<std::string> missing_symbols;
-    return DPMError::SUCCESS;
+    DPMError validate_error = validate_module_interface(module_handle, missing_symbols);
+    if ( validate_error != DPMError::SUCCESS ) {
+        dlclose(module_handle);
+        return validate_error;
+    }
+
+    return validate_error;
 }
 
 DPMError ModuleLoader::execute_module(const std::string& module_name, const std::string& command) const
 {
-    void* module_handle;
+    // declare a module_handle
-    DPMError load_error = load_module(module_name, module_handle);
+    void * module_handle;
 
-    if (load_error != DPMError::SUCCESS) {
+    // attempt to load the module
+    DPMError load_error = load_module( module_name, module_handle );
+    if ( load_error != DPMError::SUCCESS ) {
         return load_error;
     }
 
-    std::vector<std::string> missing_symbols;
+    // Clear any previous error state and handle any residual failure
-    DPMError validate_error = validate_module_interface(module_handle, missing_symbols);
+    const char* pre_error = dlerror();
-    if (validate_error != DPMError::SUCCESS) {
+    if ( pre_error != nullptr ) {
-        dlclose(module_handle);
+        dlclose( module_handle );
-        return DPMError::INVALID_MODULE;
+        return DPMError::UNDEFINED_ERROR;
     }
 
-    using ExecuteFn = int (*)(const char*, int, char**);
+    // declare a function pointer type to hold the module symbol to execute
-    ExecuteFn execute_fn = (ExecuteFn)dlsym(module_handle, "dpm_module_execute");
+    typedef int (*ExecuteFn) ( const char*, int, char** );
 
-    const char* error = dlerror();
+    // populate that void pointer to the execute symbol in the module with
-    DPMError result = DPMError::SUCCESS;
+    ExecuteFn execute_fn = (ExecuteFn) dlsym( module_handle, "dpm_module_execute" );
 
-    if (error != nullptr) {
+    // do basic error handling to detect if the symbol look up was successful
-        result = DPMError::MODULE_LOAD_FAILED;
+    const char * dlsym_error = dlerror();
-    } else {
+    if ( dlsym_error != nullptr ) {
-        execute_fn(command.c_str(), 0, nullptr);
+        dlclose( module_handle );
+        return DPMError::SYMBOL_NOT_FOUND;
     }
 
-    dlclose(module_handle);
+    // check if the void pointer was populated
-    return result;
+    if ( execute_fn == nullptr ) {
+        dlclose( module_handle );
+        return DPMError::SYMBOL_NOT_FOUND;
+    }
+
+    // execute the symbol that was loaded and supply the command string being routed from DPM
+    int exec_error = execute_fn( command.c_str(), 0, nullptr );
+
+    // irregardless of result, this is the time to close the module handle
+    dlclose( module_handle );
+
+    // if the result of execution was not 0, return an error
+    if ( exec_error != 0 ) {
+        return DPMError::SYMBOL_EXECUTION_FAILED;
+    }
+
+    // if we made it here, assume it was successful
+    return DPMError::SUCCESS;
 }
 
-DPMError ModuleLoader::get_module_version(void* module_handle, std::string& version) const
+
+DPMError ModuleLoader::get_module_version( void * module_handle, std::string& version ) const
 {
-    if (!module_handle) {
+    // validate that the module is even loaded
+    if ( !module_handle ) {
+        version = "DPM ERROR";
+        return DPMError::MODULE_NOT_LOADED;
+    }
+
+    // Clear any previous error state and handle any residual failure
+    const char* pre_error = dlerror();
+    if ( pre_error != nullptr ) {
+        version = pre_error;
+        return DPMError::UNDEFINED_ERROR;
+    }
+
+    // declare a function pointer type to hold the module symbol to execute
+    typedef const char * (* VersionFn)();
+
+    // populate that void pointer to execute the symbol in the module with
+    VersionFn version_fn = (VersionFn) dlsym( module_handle, "dpm_module_get_version" );
+
+    // Check for errors from dlsym
+    const char* error = dlerror();
+    if (error != nullptr) {
+        version = error;
+        return DPMError::SYMBOL_NOT_FOUND;
+    }
+
+    // check if the void pointer was populated
+    if ( version_fn == nullptr ) {
         version = "ERROR";
+        return DPMError::SYMBOL_NOT_FOUND;
+    }
+
+    // execute the loaded symbol
+    const char * ver = version_fn();
+
+    // check the return, and throw an error if it's a null value
+    if ( ver == nullptr ) {
+        version = "MODULE ERROR";
         return DPMError::INVALID_MODULE;
     }
 
-    dlerror();
+    // if you made it here, assume success
-
-    using GetVersionFn = const char* (*)();
-    GetVersionFn get_version = (GetVersionFn)dlsym(module_handle, "dpm_module_get_version");
-
-    const char* error = dlerror();
-    if (error != nullptr) {
-        version = "unknown";
-        return DPMError::MODULE_LOAD_FAILED;
-    }
-
-    const char* ver = get_version();
-    version = ver ? ver : "unknown";
     return DPMError::SUCCESS;
 }
 
 DPMError ModuleLoader::get_module_description(void* module_handle, std::string& description) const
 {
+    // validate that the module is even loaded
     if (!module_handle) {
+        description = "DPM ERROR";
+        return DPMError::MODULE_NOT_LOADED;
+    }
+
+    // Clear any previous error state and handle any residual failure
+    const char* pre_error = dlerror();
+    if ( pre_error != nullptr ) {
+        version = pre_error;
+        return DPMError::UNDEFINED_ERROR;
+    }
+
+    // declare a function pointer type to hold the module symbol to execute
+    typedef const char * (* DescriptionFn)();
+
+    // populate that void pointer to execute the symbol in the module with
+    DescriptionFn description_fn = (DescriptionFn) dlsym( module_handle, "dpm_get_description" );
+
+    // check for errors from dlsym
+    const char* error = dlerror();
+    if ( error != nullptr ) {
+        description = "ERROR";
+        return DPMError::SYMBOL_NOT_FOUND;
+    }
+
+    // check if the void pointer was populated
+    if ( description_fn == nullptr ) {
         description = "ERROR";
         return DPMError::INVALID_MODULE;
     }
 
-    dlerror();
+    // execute the loaded symbol
+    const char* desc = description_fn();
 
-    using GetDescriptionFn = const char* (*)();
+    // check the return, and throw an error if it's a null value
-    GetDescriptionFn get_description = (GetDescriptionFn)dlsym(module_handle, "dpm_get_description");
+    if ( desc == nullptr ) {
-
+        description = "MODULE ERROR";
-    const char* error = dlerror();
+        return DPMError::INVALID_MODULE;
-    if (error != nullptr) {
-        description = "unknown";
-        return DPMError::MODULE_LOAD_FAILED;
     }
 
-    const char* desc = get_description();
+    // if you made it here, assume success
-    description = desc ? desc : "unknown";
     return DPMError::SUCCESS;
 }
 
 DPMError ModuleLoader::validate_module_interface(void* module_handle, std::vector<std::string>& missing_symbols) const
 {
-    if (!module_handle) {
+    // validate that the module is even loaded
-        return DPMError::INVALID_MODULE;
+    if ( !module_handle ) {
+        return DPMError::MODULE_NOT_LOADED;
     }
 
+    // ensure our starting point of missing symbols is empty
     missing_symbols.clear();
 
+    // get the size of the loop (should be equal to the number of required symbols)
     size_t num_symbols = module_interface::required_symbols.size();
     for (size_t i = 0; i < num_symbols; i++) {
         dlerror();

src/dpm.cpp

@@ -46,5 +46,5 @@ int main( int argc, char* argv[] )
     DPMError execute_error = loader.execute_module(args.module_name, args.command);
 
     // pair result with a message and exit with the appropriate error code
-    return print_error(execute_error, args.module_name, args.module_path);
+    return print_error( execute_error, args.module_name, args.module_path );
 }

src/dpm_interface.cpp

@@ -62,9 +62,9 @@ int main_list_modules(const ModuleLoader& loader)
     }
 
     std::vector<std::string> valid_modules;
-    for (const auto& module : modules) {
+    for (int i = 0; i < modules.size(); i++) {
         void* handle;
-        DPMError load_error = loader.load_module(module, handle);
+        DPMError load_error = loader.load_module(modules[i], handle);
         if (load_error != DPMError::SUCCESS) {
             continue;
         }
@@ -72,7 +72,7 @@ int main_list_modules(const ModuleLoader& loader)
         std::vector<std::string> missing_symbols;
         DPMError validate_error = loader.validate_module_interface(handle, missing_symbols);
         if (validate_error == DPMError::SUCCESS) {
-            valid_modules.push_back(module);
+            valid_modules.push_back(modules[i]);
         }
         dlclose(handle);
     }
@@ -84,12 +84,12 @@ int main_list_modules(const ModuleLoader& loader)
 
     size_t max_name_length = 0;
     size_t max_version_length = 0;
-    for (const auto& module : valid_modules) {
+    for (int i = 0; i < valid_modules.size(); i++) {
         void* module_handle;
         std::string version;
-        max_name_length = std::max(max_name_length, module.length());
+        max_name_length = std::max(max_name_length, valid_modules[i].length());
 
-        DPMError load_error = loader.load_module(module, module_handle);
+        DPMError load_error = loader.load_module(valid_modules[i], module_handle);
         if (load_error == DPMError::SUCCESS) {
             DPMError version_error = loader.get_module_version(module_handle, version);
             if (version_error == DPMError::SUCCESS) {
@@ -101,24 +101,24 @@ int main_list_modules(const ModuleLoader& loader)
 
     const int column_spacing = 4;
 
-    std::cout << "Available modules in '" << path << "':" << std::endl << std::endl;
+    std::cout << "\nAvailable modules in '" << path << "':" << std::endl << std::endl;
     std::cout << std::left << std::setw(max_name_length + column_spacing) << "MODULE"
               << std::setw(max_version_length + column_spacing) << "VERSION"
               << "DESCRIPTION" << std::endl;
 
-    for (const auto& module_name : valid_modules) {
+    for (int i = 0; i < valid_modules.size(); i++) {
         void* module_handle;
         std::string version = "unknown";
         std::string description = "unknown";
 
-        DPMError load_error = loader.load_module(module_name, module_handle);
+        DPMError load_error = loader.load_module(valid_modules[i], module_handle);
         if (load_error == DPMError::SUCCESS) {
             DPMError version_error = loader.get_module_version(module_handle, version);
             DPMError desc_error = loader.get_module_description(module_handle, description);
             dlclose(module_handle);
         }
 
-        std::cout << std::left << std::setw(max_name_length + column_spacing) << module_name
+        std::cout << std::left << std::setw(max_name_length + column_spacing) << valid_modules[i]
                   << std::setw(max_version_length + column_spacing) << version
                   << description << std::endl;
     }
@@ -143,14 +143,14 @@ CommandArgs parse_args(int argc, char* argv[])
         switch (opt) {
             case 'm':
                 args.module_path = optarg;
-            break;
+                break;
             case 'h':
                 std::cout << "Usage: dpm [options] [module-name] [module args...]\n\n"
-                         << "Options:\n\n"
+                          << "Options:\n\n"
-                         << "  -m, --module-path PATH   Path to DPM modules\n"
+                          << "  -m, --module-path PATH   Path to DPM modules\n"
-                         << "  -h, --help              Show this help message\n"
+                          << "  -h, --help              Show this help message\n\n"
-                         << "\nIf no module is specified, available modules will be listed.\n\n";
+                          << "If no module is specified, available modules will be listed.\n\n";
-            exit(0);
+                exit(0);
             case '?':
                 exit(1);
         }
@@ -192,12 +192,18 @@ int print_error(DPMError error, const std::string& module_name, const std::strin
         case DPMError::MODULE_NOT_FOUND:
             std::cerr << "Module not found: " << module_name << std::endl;
         return 1;
+        case DPMError::MODULE_NOT_LOADED:
+            std::cerr << "Attempted to execute module before loading it: " << module_name << std::endl;
+        return 1;
         case DPMError::MODULE_LOAD_FAILED:
             std::cerr << "Failed to load module: " << module_name << std::endl;
         return 1;
         case DPMError::INVALID_MODULE:
             std::cerr << "Invalid module format: " << module_name << std::endl;
         return 1;
+        case DPMError::UNDEFINED_ERROR:
+            std::cerr << "Undefined error occurred with module: " << module_name << std::endl;
+        return 1;
         default:
             std::cerr << "Unknown error executing module: " << module_name << std::endl;
         return 1;