[Windows] clean up the setting of environment variables cross-platform (#2655)

* clean up the setting of environment variables cross-platform

* fix clang-tidy

Author: apwojcik

test/env_utils.hpp

@@ -0,0 +1,62 @@
+/*******************************************************************************
+ *
+ * MIT License
+ *
+ * Copyright (c) 2017 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ *******************************************************************************/
+
+#ifndef GUARD_MIOPEN_ENV_UTILS_HPP
+#define GUARD_MIOPEN_ENV_UTILS_HPP
+
+#ifdef _WIN32
+#define WIN32_LEAN_AND_MEAN
+#include <Windows.h>
+#endif
+
+#include "test.hpp"
+#include <string_view>
+
+inline void setEnvironmentVariable(std::string_view name, std::string_view value)
+{
+#ifdef _WIN32
+    BOOL ret = SetEnvironmentVariable(name.data(), value.data());
+    EXPECT_EQUAL(ret, TRUE);
+#else
+    // NOLINTNEXTLINE(concurrency-mt-unsafe)
+    int ret = setenv(name.data(), value.data(), 1);
+    EXPECT_EQUAL(ret, 0);
+#endif
+}
+
+inline void unsetEnvironmentVariable(std::string_view name)
+{
+#ifdef _WIN32
+    BOOL ret = SetEnvironmentVariable(name.data(), nullptr);
+    EXPECT_EQUAL(ret, TRUE);
+#else
+    // NOLINTNEXTLINE(concurrency-mt-unsafe)
+    int ret = unsetenv(name.data());
+    EXPECT_EQUAL(ret, 0);
+#endif
+}
+
+#endif // GUARD_MIOPEN_ENV_UTILS_HPP

test/find_db.cpp

@@ -28,6 +28,7 @@
 #include "driver.hpp"
 #include "get_handle.hpp"
 #include "workspace.hpp"
+#include "env_utils.hpp"
 
 #include <miopen/convolution.hpp>
 #include <miopen/conv/problem_description.hpp>
@@ -38,12 +39,6 @@
 #include <miopen/hip_build_utils.hpp>
 
 #include <chrono>
-#ifdef _WIN32
-#define WIN32_LEAN_AND_MEAN
-#include <Windows.h>
-#else
-#include <cstdlib>
-#endif
 #include <functional>
 
 namespace miopen {
@@ -229,14 +224,8 @@ struct FindDbTest : test_driver
 
 int main(int argc, const char* argv[])
 {
-#ifdef _WIN32
-    SetEnvironmentVariable("MIOPEN_LOG_LEVEL", "6");
-    SetEnvironmentVariable("MIOPEN_COMPILE_PARALLEL_LEVEL", "1");
-    SetEnvironmentVariable("MIOPEN_ENABLE_LOGGING_ELAPSED_TIME", "1");
-#else
-    setenv("MIOPEN_LOG_LEVEL", "6", 1);                   // NOLINT (concurrency-mt-unsafe)
-    setenv("MIOPEN_COMPILE_PARALLEL_LEVEL", "1", 1);      // NOLINT (concurrency-mt-unsafe)
-    setenv("MIOPEN_ENABLE_LOGGING_ELAPSED_TIME", "1", 1); // NOLINT (concurrency-mt-unsafe)
-#endif
+    setEnvironmentVariable("MIOPEN_LOG_LEVEL", "6");
+    setEnvironmentVariable("MIOPEN_COMPILE_PARALLEL_LEVEL", "1");
+    setEnvironmentVariable("MIOPEN_ENABLE_LOGGING_ELAPSED_TIME", "1");
     test_drive<miopen::FindDbTest>(argc, argv);
 }

test/gtest/bad_fusion_plan.cpp

@@ -30,24 +30,12 @@
 #include "tensor_holder.hpp"
 #include "get_handle.hpp"
 #include "conv_test_base.hpp"
+#include "../env_utils.hpp"
 
 #if MIOPEN_BACKEND_HIP
 
 namespace bad_fusion_plan {
 
-void setEnvironmentVariable(const std::string& name, const std::string& value)
-{
-    int ret = 0;
-
-#ifdef _WIN32
-    std::string env_var(name + "=" + value);
-    ret = _putenv(env_var.c_str());
-#else
-    ret = setenv(name.c_str(), value.c_str(), 1);
-#endif
-    EXPECT_EQ(ret, 0);
-}
-
 struct ConvTestCaseFusion
 {
     size_t N;

test/gtest/cba_infer.cpp

@@ -35,6 +35,7 @@
 #include "tensor_util.hpp"
 #include "get_handle.hpp"
 #include "cba.hpp"
+#include "../env_utils.hpp"
 
 namespace cba_infer {
 
@@ -50,19 +51,6 @@ struct ConvBiasActivInferTestHalf : ConvBiasActivInferTest<half_float::half>
 {
 };
 
-void setEnvironmentVariable(const std::string& name, const std::string& value)
-{
-    int ret = 0;
-
-#ifdef _WIN32
-    std::string env_var(name + "=" + value);
-    ret = _putenv(env_var.c_str());
-#else
-    ret = setenv(name.c_str(), value.c_str(), 1);
-#endif
-    EXPECT_EQ(ret, 0);
-}
-
 template <typename Solver, typename TestCase>
 void RunSolver(miopen::FusionPlanDescriptor& fusePlanDesc,
                const std::unique_ptr<miopen::fusion::FusionInvokeParams>& plan_params,

test/gtest/db_sync.cpp

@@ -46,6 +46,11 @@
 
 #define WORKAROUND_ISSUE_2493 1
 
+#if WORKAROUND_ISSUE_2493 && defined(_WIN32)
+#define WIN32_LEAN_AND_MEAN
+#include <Windows.h>
+#endif
+
 #define WORKAROUND_ISSUE_1987 0      // Allows testing FDB on gfx1030 (legacy fdb).
 #define SKIP_KDB_PDB_TESTING 0       // Allows testing FDB on gfx1030.
 #define SKIP_CONVOCLDIRECTFWDFUSED 0 // Allows testing FDB on gfx1030 (legacy fdb).
@@ -74,14 +79,10 @@ struct std::hash<KDBKey>
     }
 };
 
-#if WORKAROUND_ISSUE_2493
-static void SetEnvironmentVariable(const std::string& name, const std::string& value)
+#if WORKAROUND_ISSUE_2493 && !defined(_WIN32)
+static void SetEnvironmentVariable(std::string_view name, std::string_view value)
 {
-#ifdef _WIN32
-    const auto ret = _putenv_s(env_var.c_str(), value.c_str());
-#else
-    const auto ret = setenv(name.c_str(), value.c_str(), 1);
-#endif
+    const auto ret = setenv(name.data(), value.data(), 1);
     ASSERT_TRUE(ret == 0);
 }
 #endif // WORKAROUND_ISSUE_2493

test/gtest/log.cpp

@@ -29,8 +29,8 @@
 
 #include <miopen/config.h>
 #include <miopen/fusion_plan.hpp>
-#include <cstdlib>
 #include "../random.hpp"
+#include "../env_utils.hpp"
 
 #if MIOPEN_BACKEND_OPENCL
 #define BKEND "OpenCL"
@@ -57,31 +57,6 @@ const std::string logBnormActiv =
 
 const std::string envConv = "MIOPEN_ENABLE_LOGGING_CMD";
 
-static void setEnvironmentVariable(const std::string& name, const std::string& value)
-{
-    int ret = 0;
-
-#ifdef _WIN32
-    std::string env_var(name + "=" + value);
-    ret = _putenv(env_var.c_str());
-#else
-    ret = setenv(name.c_str(), value.c_str(), 1);
-#endif
-    EXPECT_EQ(ret, 0);
-}
-
-static void unSetEnvironmentVariable(const std::string& name)
-{
-    int ret = 0;
-#ifdef _WIN32
-    std::string empty_env_var(name + "=");
-    ret = _putenv(empty_env_var.c_str());
-#else
-    ret = unsetenv(name.c_str());
-#endif
-    EXPECT_EQ(ret, 0);
-}
-
 // Captures the std::cerr buffer and store it to a string.
 struct CerrRedirect
 {
@@ -290,7 +265,7 @@ void TestLogFun(std::function<void(const miopenTensorDescriptor_t&,
     if(set_env)
         setEnvironmentVariable(env_var, "1");
     else
-        unSetEnvironmentVariable(env_var);
+        unsetEnvironmentVariable(env_var);
 
     func(test_conv_log.input.desc,
          test_conv_log.weights.desc,
@@ -316,7 +291,7 @@ void TestLogCmdCBAFusion(std::function<void(const miopenFusionPlanDescriptor_t)>
     if(set_env)
         setEnvironmentVariable(env_var, "1");
     else
-        unSetEnvironmentVariable(env_var);
+        unsetEnvironmentVariable(env_var);
 
     CreateCBAFusionPlan fp_cba_create;
     fp_cba_create.CBAPlan();
@@ -341,7 +316,7 @@ void TestLogCmdBNormFusion(std::function<void(const miopenFusionPlanDescriptor_t
     if(set_env)
         setEnvironmentVariable(env_var, "1");
     else
-        unSetEnvironmentVariable(env_var);
+        unsetEnvironmentVariable(env_var);
 
     CreateBNormFusionPlan<float> fp_bnorm_create;
     fp_bnorm_create.BNormActivation();