Rewrote local char* to use std::unique_ptr.

Author: Petter Reinholdtsen

src/tbb/cgroup_info.h

@@ -150,42 +150,27 @@ class cgroup_info {
 
     static bool try_read_cgroup_v1_num_cpus_from(const char* dir, int& num_cpus) {
         std::size_t pathlen = strlen(dir) + 30;
-        char *path = (char*)calloc(pathlen, 1);
-        if (std::snprintf(path, pathlen, "%s/cpu.cfs_quota_us", dir) < 0) {
-            free(path);
-            path = NULL;
+        std::unique_ptr<char[]> path(new char[pathlen]);
+        if (std::snprintf(path.get(), pathlen, "%s/cpu.cfs_quota_us", dir) < 0)
             return false;       // Failed to create path
-        }
 
-        unique_file_t fd(std::fopen(path, "r"), &close_file);
-        if (!fd) {
-            free(path);
-            path = NULL;
+        unique_file_t fd(std::fopen(path.get(), "r"), &close_file);
+        if (!fd)
             return false;
-        }
 
         long long cpu_quota = 0;
-        if (std::fscanf(fd.get(), "%lld", &cpu_quota) != 1) {
-            free(path);
-            path = NULL;
+        if (std::fscanf(fd.get(), "%lld", &cpu_quota) != 1)
             return false;
-        }
 
         if (-1 == cpu_quota) {
             num_cpus = unlimited_num_cpus; // -1 quota means maximum available CPUs
-            free(path);
-            path = NULL;
             return true;
         }
 
-        if (std::snprintf(path, pathlen, "%s/cpu.cfs_period_us", dir) < 0) {
-            free(path);
-            path = NULL;
+        if (std::snprintf(path.get(), pathlen, "%s/cpu.cfs_period_us", dir) < 0)
             return false;       // Failed to create path;
-        }
-        fd.reset(std::fopen(path, "r"));
-        free(path);
-        path = NULL;
+
+        fd.reset(std::fopen(path.get(), "r"));
         if (!fd)
             return false;
 
@@ -199,16 +184,11 @@ class cgroup_info {
 
     static bool try_read_cgroup_v2_num_cpus_from(const char* dir, int& num_cpus) {
         std::size_t pathlen = strlen(dir) + 30;
-        char *path = (char*)calloc(pathlen, 1);
-        if (std::snprintf(path, pathlen, "%s/cpu.max", dir) < 0) {
-            free(path);
-            path = NULL;
+        std::unique_ptr<char[]> path(new char[pathlen]);
+        if (std::snprintf(path.get(), pathlen, "%s/cpu.max", dir) < 0)
             return false;       // Failed to create path
-        }
 
-        unique_file_t fd(std::fopen(path, "r"), &close_file);
-        free(path);
-        path = NULL;
+        unique_file_t fd(std::fopen(path.get(), "r"), &close_file);
         if (!fd)
             return false;
 
@@ -246,18 +226,14 @@ class cgroup_info {
     static int parse_cgroup_entry(const char* mnt_dir, process_cgroup_data& pcd) {
         int num_cpus = error_value; // Initialize to an impossible value
         std::size_t dirlen = strlen(mnt_dir) + strlen(pcd.relative_path) + 2;
-        char *dir = (char*)calloc(dirlen, 1);
-        if (std::snprintf(dir, dirlen, "%s/%s", mnt_dir, pcd.relative_path) >= 0) {
-            if (try_read_cgroup_num_cpus_from(dir, num_cpus, pcd.version)) {
-                free(dir);
-                dir = NULL;
+        std::unique_ptr<char[]> dir(new char[dirlen]);
+        if (std::snprintf(dir.get(), dirlen, "%s/%s", mnt_dir, pcd.relative_path) >= 0) {
+            if (try_read_cgroup_num_cpus_from(dir.get(), num_cpus, pcd.version)) {
                 return num_cpus;
             }
         }
-        num_cpus = try_read_cgroup_num_cpus_from(mnt_dir, num_cpus, pcd.version) ? num_cpus : error_value;
-        free(dir);
-        dir = NULL;
-        return num_cpus;
+
+        return try_read_cgroup_num_cpus_from(mnt_dir, num_cpus, pcd.version) ? num_cpus : error_value;
     }
 
     static bool is_cpu_restriction_possible(process_cgroup_data& pcd) {
@@ -307,19 +283,18 @@ class cgroup_info {
         std::size_t dirlen = (strlen(pcd.relative_path)
                               + strlen(cg_cfg.sys_fs_cgroup_dir_path)
                               + 25);
-        char *dir = (char*)calloc(dirlen, 1);
+        std::unique_ptr<char[]> dir(new char[dirlen]);
         __TBB_ASSERT(*pcd.relative_path, nullptr);
-        if (0 <= std::snprintf(dir, dirlen, "%s/%s", cg_cfg.sys_fs_cgroup_dir_path,
+        if (0 <= std::snprintf(dir.get(), dirlen, "%s/%s",
+                               cg_cfg.sys_fs_cgroup_dir_path,
                                pcd.relative_path))
-            try_read_cgroup_num_cpus_from(dir, num_cpus, pcd.version);
+            try_read_cgroup_num_cpus_from(dir.get(), num_cpus, pcd.version);
 
         if (error_value == num_cpus && pcd.version == process_cgroup_data::cgroup_version::v2) {
-            if (0 <= std::snprintf(dir, dirlen, "%s/%s", "/sys/fs/cgroup/unified",
+            if (0 <= std::snprintf(dir.get(), dirlen, "%s/%s", "/sys/fs/cgroup/unified",
                                    pcd.relative_path))
-                try_read_cgroup_v2_num_cpus_from(dir, num_cpus);
+                try_read_cgroup_v2_num_cpus_from(dir.get(), num_cpus);
         }
-        free(dir);
-        dir = NULL;
         return num_cpus;
     }
 

src/tbb/dynamic_link.cpp

@@ -690,22 +690,18 @@ namespace r1 {
      *         occurs, in which case the error is optionally reported.
      */
     bool has_valid_signature(const char* filepath, const std::size_t length) {
-        std::size_t pathlen = strlen(filepath)+1;
-        wchar_t *wfilepath = (wchar_t*)calloc(pathlen, sizeof(wchar_t));
+        std::unique_ptr<wchar_t[]> path(new wchar_t[length+1]);
         {
             std::mbstate_t state{};
             const char* ansi_filepath = filepath; // mbsrtowcs moves original pointer
-            const size_t num_converted = mbsrtowcs(wfilepath, &ansi_filepath, length, &state);
-            if (num_converted == std::size_t(-1)) {
-                free (wfilepath);
-                wfilepath = NULL;
+            const size_t num_converted = mbsrtowcs(wfilepath.get(), &ansi_filepath, length, &state);
+            if (num_converted == std::size_t(-1))
                 return false;
-            }
         }
         WINTRUST_FILE_INFO fdata;
         std::memset(&fdata, 0, sizeof(fdata));
         fdata.cbStruct       = sizeof(WINTRUST_FILE_INFO);
-        fdata.pcwszFilePath  = wfilepath;
+        fdata.pcwszFilePath  = wfilepath.get();
 
         // Check that the certificate used to sign the specified file chains up to a root
         // certificate located in the trusted root certificate store, implying that the identity of
@@ -730,8 +726,6 @@ namespace r1 {
         pWVTData.dwStateAction = WTD_STATEACTION_CLOSE;       // Release WVT state data
         (void)WinVerifyTrust(NULL, &pgActionID, &pWVTData);
 
-        free (wfilepath);
-        wfilepath = NULL;
         return ERROR_SUCCESS == rc;
     }
 #endif  // _WIN32 && !__TBB_SKIP_DEPENDENCY_SIGNATURE_VERIFICATION
@@ -744,49 +738,38 @@ namespace r1 {
 #if __TBB_DYNAMIC_LOAD_ENABLED
         const char* path = library;
         std::size_t len = abs_path(library, NULL, 1);
-        char *absolute_path = (char*)calloc(len, 1);
+        std::unique_ptr<char[]> absolute_path(new char[len]);
         std::size_t length = 0;
         const bool build_absolute_path = flags & DYNAMIC_LINK_BUILD_ABSOLUTE_PATH;
         if (build_absolute_path) {
-            length = abs_path( library, absolute_path, len );
+            length = abs_path( library, absolute_path.get(), len );
             if (length == 0) {
                 // length == 0 means failing of init_ap_data so the warning has already been issued.
-                free(absolute_path);
-                absolute_path = NULL;
                 return nullptr;
-            } else if (!file_exists(absolute_path)) {
+            } else if (!file_exists(absolute_path.get())) {
                 // Path to a file has been built manually. It is not proven to exist however.
-                DYNAMIC_LINK_WARNING( dl_lib_not_found, absolute_path, dlerror() );
-                free(absolute_path);
-                absolute_path = NULL;
+                DYNAMIC_LINK_WARNING( dl_lib_not_found, absolute_path.get(), dlerror() );
                 return nullptr;
             }
-            path = absolute_path;
+            path = absolute_path.get();
         }
 #if _WIN32
 #if !__TBB_SKIP_DEPENDENCY_SIGNATURE_VERIFICATION
         if (!build_absolute_path) { // Get the path if it is not yet built
-            length = get_module_path(absolute_path, (unsigned)len, library);
+            length = get_module_path(absolute_path.get(), (unsigned)len, library);
             if (length == 0) {
                 DYNAMIC_LINK_WARNING( dl_lib_not_found, path, dlerror() );
-                free(absolute_path);
-                absolute_path = NULL;
                 return library_handle;
             } else if (length >= len) { // The buffer length was insufficient
                 DYNAMIC_LINK_WARNING( dl_buff_too_small );
-                free(absolute_path);
-                absolute_path = NULL;
                 return library_handle;
             }
             length += 1;   // Count terminating NULL character as part of string length
-            path = absolute_path;
+            path = absolute_path.get();
         }
 
-        if (!has_valid_signature(path, length)) {
-            free(absolute_path);
-            absolute_path = NULL;
+        if (!has_valid_signature(path, length))
             return library_handle; // Warning (if any) has already been reported
-        }
 #endif /* !__TBB_SKIP_DEPENDENCY_SIGNATURE_VERIFICATION */
         // Prevent Windows from displaying silly message boxes if it fails to load library
         // (e.g. because of MS runtime problems - one of those crazy manifest related ones)
@@ -805,8 +788,6 @@ namespace r1 {
             }
         } else
             DYNAMIC_LINK_WARNING( dl_lib_not_found, path, dlerror() );
-        free(absolute_path);
-        absolute_path = NULL;
 #endif /* __TBB_DYNAMIC_LOAD_ENABLED */
         return library_handle;
     }

test/common/utils_concurrency_limit.h

@@ -208,43 +208,27 @@ class cgroup_info {
 
     static bool try_read_cgroup_v1_num_cpus_from(const char* dir, int& num_cpus) {
         std::size_t pathlen = strlen(dir) + 20;
-        char *path = (char*)calloc(pathlen, 1);
-        if (std::snprintf(path, pathlen, "%s/cpu.cfs_quota_us", dir) < 0) {
-            free(path);
-            path = NULL;
+        std::unique_ptr<char[]> path(new char[pathlen]);
+        if (std::snprintf(path.get(), pathlen, "%s/cpu.cfs_quota_us", dir) < 0)
             return false; // Failed to create path
-        }
 
-        unique_file_t fd(std::fopen(path, "r"), &close_file);
-        if (!fd) {
-            free(path);
-            path = NULL;
+        unique_file_t fd(std::fopen(path.get(), "r"), &close_file);
+        if (!fd)
             return false;
-        }
 
         long long cpu_quota = 0;
-        if (std::fscanf(fd.get(), "%lld", &cpu_quota) != 1) {
-            free(path);
-            path = NULL;
+        if (std::fscanf(fd.get(), "%lld", &cpu_quota) != 1)
             return false;
-        }
 
         if (-1 == cpu_quota) {
             num_cpus = unlimited_num_cpus; // -1 quota means maximum available CPUs
-            free(path);
-            path = NULL;
             return true;
         }
 
-        if (std::snprintf(path, pathlen, "%s/cpu.cfs_period_us", dir) < 0) {
-            free(path);
-            path = NULL;
+        if (std::snprintf(path.get(), pathlen, "%s/cpu.cfs_period_us", dir) < 0)
             return false; // Failed to create path
-        }
 
-        fd.reset(std::fopen(path, "r"));
-        free(path);
-        path = NULL;
+        fd.reset(std::fopen(path.get(), "r"));
         if (!fd)
             return false;
 
@@ -258,16 +242,11 @@ class cgroup_info {
 
     static bool try_read_cgroup_v2_num_cpus_from(const char* dir, int& num_cpus) {
         std::size_t pathlen = strlen(dir) + 10;
-        char *path = (char*)calloc(pathlen, 1);
-        if (std::snprintf(path, pathlen, "%s/cpu.max", dir) < 0) {
-            free(path);
-            path = NULL;
+        std::unique_ptr<char[]> path(new char[pathlen]);
+        if (std::snprintf(path.get(), pathlen, "%s/cpu.max", dir) < 0)
             return false;
-        }
 
-        unique_file_t fd(std::fopen(path, "r"), &close_file);
-        free(path);
-        path = NULL;
+        unique_file_t fd(std::fopen(path.get(), "r"), &close_file);
         if (!fd)
             return false;
 
@@ -296,7 +275,7 @@ class cgroup_info {
     {
         int num_cpus = error_value; // Initialize to an impossible value
         std::size_t dirlen = strlen(mnt_dir) + rel_path_size + 2;
-        char *dir = NULL;
+        std::unique_ptr<char[]> dir(new char[dirlen]);
         if (!std::strncmp(mnt_type, "cgroup2", 7)) { // Found cgroup v2 mount entry
             // At first, try reading CPU quota directly
             if (try_read_cgroup_v2_num_cpus_from(mnt_dir, num_cpus))
@@ -306,11 +285,8 @@ class cgroup_info {
                 cache_relative_path_for(cgroup_fd, paths_cache);
 
             // Now try reading including relative path
-            dir = (char*)calloc(dirlen, 1);
-            if (std::snprintf(dir, dirlen, "%s/%s", mnt_dir, paths_cache.v2_relative_path) >= 0)
-                try_read_cgroup_v2_num_cpus_from(dir, num_cpus);
-            free(dir);
-            dir = NULL;
+            if (std::snprintf(dir.get(), dirlen, "%s/%s", mnt_dir, paths_cache.v2_relative_path) >= 0)
+                try_read_cgroup_v2_num_cpus_from(dir.get(), num_cpus);
             return num_cpus;
         }
 
@@ -322,11 +298,8 @@ class cgroup_info {
         if (!*paths_cache.v1_relative_path)
             cache_relative_path_for(cgroup_fd, paths_cache);
 
-        dir = (char*)calloc(dirlen, 1);
-        if (std::snprintf(dir, dirlen, "%s/%s", mnt_dir, paths_cache.v1_relative_path) >= 0)
-            try_read_cgroup_v1_num_cpus_from(dir, num_cpus);
-        free(dir);
-        dir = NULL;
+        if (std::snprintf(dir.get(), dirlen, "%s/%s", mnt_dir, paths_cache.v1_relative_path) >= 0)
+            try_read_cgroup_v1_num_cpus_from(dir.get(), num_cpus);
         return num_cpus;
     }
 

test/tbb/test_dynamic_link.cpp

@@ -167,15 +167,15 @@ TEST_CASE("Test dynamic_link with existing library") {
 TEST_CASE("Test dynamic_link with bad library") {
     const char* lib_name = TEST_LIBRARY_NAME("stub_unsigned");
     const std::size_t size = tbb::detail::r1::abs_path(lib_name, NULL, 1) + 1;
-    char *path = (char*)calloc(size, 1);
+    std::unique_ptr<char[]> path(new char[size]);
     const std::size_t msg_size = size + 128; // Path to the file + message
-    char *msg = (char*)calloc(msg_size, 1);
+    std::unique_ptr<char[]> msg(new char[msg_size]);
 
 #if !__TBB_WIN8UI_SUPPORT
-    const std::size_t len = tbb::detail::r1::abs_path(lib_name, path, size);
+    const std::size_t len = tbb::detail::r1::abs_path(lib_name, path.get(), size);
     REQUIRE_MESSAGE((0 < len && len < size), "The path to the library is not built");
-    std::snprintf(msg, msg_size, "Test prerequisite is not held - the path \"%s\" must exist", path);
-    REQUIRE_MESSAGE(tbb::detail::r1::file_exists(path), msg);
+    std::snprintf(msg.get(), msg_size, "Test prerequisite is not held - the path \"%s\" must exist", path.get());
+    REQUIRE_MESSAGE(tbb::detail::r1::file_exists(path.get()), msg.get());
 #endif
 
     // The library exists, check that it will not be loaded.
@@ -191,21 +191,17 @@ TEST_CASE("Test dynamic_link with bad library") {
     const bool link_result = tbb::detail::r1::dynamic_link(lib_name, table,
                                                            sizeof(table) / sizeof(table[0]),
                                                            /*handle*/nullptr, load_flags);
-    std::snprintf(msg, msg_size, "The library \"%s\" was loaded but should not have been.", path);
+    std::snprintf(msg.get(), msg_size, "The library \"%s\" was loaded but should not have been.", path.get());
 
     // Expectation is that the library will not be loaded because:
     // a) On Windows the library is unsigned
     // b) On Linux the library does not have exported symbols
     const bool expected_link_result = false;
 
-    REQUIRE_MESSAGE(expected_link_result == link_result, msg);
+    REQUIRE_MESSAGE(expected_link_result == link_result, msg.get());
     REQUIRE_MESSAGE(nullptr == handler, "The symbol should not be changed.");
     // TODO: Verify the warning message contains "TBB dynamic link warning: The module
     // \".*stub_unsigned.*.dll\" is unsigned or has invalid signature."
-    free(msg);
-    msg = NULL;
-    free(path);
-    path = NULL;
 }
 
 #endif // __TBB_DYNAMIC_LOAD_ENABLED