From dd6c0adc0f85a97e0e40ae8de12f1a3c07bebd3e Mon Sep 17 00:00:00 2001
From: Eric Cousineau <eric.cousineau@tri.global>
Date: Mon, 1 Apr 2019 20:19:33 -0400
Subject: [PATCH] WIP Address comments

Signed-off-by: Eric Cousineau <eric.cousineau@tri.global>
---
 src/find_library.cpp | 73 +++++++++++++++++---------------------------
 1 file changed, 28 insertions(+), 45 deletions(-)

diff --git a/src/find_library.cpp b/src/find_library.cpp
index 5abe7af..78bb4a0 100644
--- a/src/find_library.cpp
+++ b/src/find_library.cpp
@@ -14,6 +14,7 @@
 
 #include "rcpputils/find_library.hpp"
 
+#include <cassert>
 #include <cstddef>
 
 #include <list>
@@ -26,24 +27,27 @@ namespace rcpputils
 namespace
 {
 
-std::string get_env_var(const char * env_var)
-{
-  char * value = nullptr;
-#ifndef _WIN32
-  value = getenv(env_var);
-#else
-  size_t value_size;
-  _dupenv_s(&value, &value_size, env_var);
-#endif
-  std::string value_str = "";
-  if (value) {
-    value_str = value;
 #ifdef _WIN32
-    free(value);
+static constexpr char kPathVar[] = "PATH";
+static constexpr char kPathSeparator = ';';
+static constexpr char kSolibPrefix[] = "";
+static constexpr char kSolibExtension[] = ".dll";
+#elif __APPLE__
+static constexpr char kPathVar[] = "DYLD_LIBRARY_PATH";
+static constexpr char kPathSeparator = ':';
+static constexpr char kSolibPrefix[] = "lib";
+static constexpr char kSolibExtension[] = ".dylib";
+#else
+static constexpr char kPathVar[] = "LD_LIBRARY_PATH";
+static constexpr char kPathSeparator = ':';
+static constexpr char kSolibPrefix[] = "lib";
+static constexpr char kSolibExtension[] = ".so";
 #endif
-  }
-  // printf("get_env_var(%s) = %s\n", env_var, value_str.c_str());
-  return value_str;
+
+std::string get_env_var(const char * kPathVar)
+{
+  char * value = getenv(kPathVar);
+  return value ? value : "";
 }
 
 std::list<std::string> split(const std::string & value, const char delimiter)
@@ -54,14 +58,12 @@ std::list<std::string> split(const std::string & value, const char delimiter)
   while (std::getline(ss, s, delimiter)) {
     list.push_back(s);
   }
-  // printf("split(%s) = %zu\n", value.c_str(), list.size());
   return list;
 }
 
 bool is_file_exist(const char * filename)
 {
   std::ifstream h(filename);
-  // printf("is_file_exist(%s) = %s\n", filename, h.good() ? "true" : "false");
   return h.good();
 }
 
@@ -69,35 +71,16 @@ bool is_file_exist(const char * filename)
 
 std::string find_library_path(const std::string & library_name)
 {
-  // TODO(eric.cousineau): Does Poco provide this functionality?
-  const char * env_var;
-  char separator;
-  const char * filename_prefix;
-  const char * filename_extension;
-#ifdef _WIN32
-  env_var = "PATH";
-  separator = ';';
-  filename_prefix = "";
-  filename_extension = ".dll";
-#elif __APPLE__
-  env_var = "DYLD_LIBRARY_PATH";
-  separator = ':';
-  filename_prefix = "lib";
-  filename_extension = ".dylib";
-#else
-  env_var = "LD_LIBRARY_PATH";
-  separator = ':';
-  filename_prefix = "lib";
-  filename_extension = ".so";
-#endif
-  std::string search_path = get_env_var(env_var);
-  std::list<std::string> search_paths = split(search_path, separator);
+  // TODO: Does Poco provide this functionality (ros2/rcpputils#7)?
+  // TODO: Have this return a library pointer instead (ros2/rcpputils#8).
+  std::string search_path = get_env_var(kPathVar);
+  std::list<std::string> search_paths = split(search_path, kPathSeparator);
 
-  std::string filename = filename_prefix;
-  filename += library_name + filename_extension;
+  std::string filename = kSolibPrefix;
+  filename += library_name + kSolibExtension;
 
-  for (auto it : search_paths) {
-    std::string path = it + "/" + filename;
+  for (const auto& search_path : search_paths) {
+    std::string path = search_path + "/" + filename;
     if (is_file_exist(path.c_str())) {
       return path;
     }