Skip to content

Commit aee9492

Browse files
fix: replace UT_ASSERTs with GTEST asserts
If possible, change non-void functions to void by passing pointers Otherwise, introduce error-indicating return values Fix segfault in umfMemoryProviderDestroy(...) Ref. #569
1 parent 5d270d2 commit aee9492

File tree

3 files changed

+51
-14
lines changed

3 files changed

+51
-14
lines changed

src/memory_provider.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,10 @@ umf_result_t umfMemoryProviderCreate(const umf_memory_provider_ops_t *ops,
195195
}
196196

197197
void umfMemoryProviderDestroy(umf_memory_provider_handle_t hProvider) {
198-
hProvider->ops.finalize(hProvider->provider_priv);
199-
umf_ba_global_free(hProvider);
198+
if (hProvider) {
199+
hProvider->ops.finalize(hProvider->provider_priv);
200+
umf_ba_global_free(hProvider);
201+
}
200202
}
201203

202204
static void

test/memspaces/memspace_highest_capacity.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ struct memspaceHighestCapacityProviderTest : ::numaNodesTest {
2121
::numaNodesTest::SetUp();
2222

2323
umf_const_memspace_handle_t hMemspace = umfMemspaceHighestCapacityGet();
24-
UT_ASSERTne(hMemspace, nullptr);
24+
ASSERT_NE(hMemspace, nullptr);
2525

2626
umf_result_t ret =
2727
umfMemoryProviderCreateFromMemspace(hMemspace, nullptr, &hProvider);

test/provider_os_memory_multiple_numa_nodes.cpp

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,14 @@ std::vector<int> get_available_cpus() {
4242
CPU_ZERO(mask);
4343

4444
int ret = sched_getaffinity(0, sizeof(cpu_set_t), mask);
45-
UT_ASSERTeq(ret, 0);
45+
46+
if (ret != 0) {
47+
available_cpus.emplace_back(-__LINE__);
48+
CPU_FREE(mask);
49+
50+
return available_cpus;
51+
}
52+
4653
// Get all available cpus.
4754
printf("All CPUs: ");
4855
for (size_t i = 0; i < CPU_SETSIZE; ++i) {
@@ -88,13 +95,21 @@ struct testNuma : testing::Test {
8895
ASSERT_NE(os_memory_provider, nullptr);
8996
}
9097

91-
struct bitmask *retrieve_nodemask(void *addr) {
98+
std::pair<int, bitmask *> retrieve_nodemask(void *addr) {
9299
struct bitmask *retrieved_nodemask = numa_allocate_nodemask();
93-
UT_ASSERTne(nodemask, nullptr);
100+
101+
if (nodemask == nullptr) {
102+
return std::make_pair(__LINE__, nodemask);
103+
}
104+
94105
int ret = get_mempolicy(nullptr, retrieved_nodemask->maskp,
95106
nodemask->size, addr, MPOL_F_ADDR);
96-
UT_ASSERTeq(ret, 0);
97-
return retrieved_nodemask;
107+
108+
if (ret != 0) {
109+
return std::make_pair(__LINE__, retrieved_nodemask);
110+
}
111+
112+
return std::pair(0, retrieved_nodemask);
98113
}
99114

100115
void TearDown() override {
@@ -241,7 +256,18 @@ TEST_P(testNumaOnEachNode, checkModeInterleaveSingleNode) {
241256
EXPECT_NODE_EQ(ptr, numa_node_number);
242257
}
243258

244-
struct testNumaOnEachCpu : testNuma, testing::WithParamInterface<int> {};
259+
struct testNumaOnEachCpu : testNuma, testing::WithParamInterface<int> {
260+
void SetUp() override {
261+
::testNuma::SetUp();
262+
263+
int cpuNumber = this->GetParam();
264+
265+
if (cpuNumber < 0) {
266+
GTEST_FAIL() << "ret is not equal to 0 in " << __FILE__ << ": "
267+
<< -cpuNumber;
268+
}
269+
}
270+
};
245271

246272
INSTANTIATE_TEST_SUITE_P(testNumaNodesAllocationsAllCpus, testNumaOnEachCpu,
247273
::testing::ValuesIn(get_available_cpus()));
@@ -260,7 +286,7 @@ TEST_P(testNumaOnEachCpu, checkModePreferredEmptyNodeset) {
260286
int ret = sched_setaffinity(0, sizeof(cpu_set_t), mask);
261287
CPU_FREE(mask);
262288

263-
UT_ASSERTeq(ret, 0);
289+
ASSERT_EQ(ret, 0);
264290

265291
umf_os_memory_provider_params_t os_memory_provider_params =
266292
UMF_OS_MEMORY_PROVIDER_PARAMS_TEST;
@@ -275,7 +301,7 @@ TEST_P(testNumaOnEachCpu, checkModePreferredEmptyNodeset) {
275301

276302
// Verify we're on the expected CPU
277303
int cpu_check = sched_getcpu();
278-
UT_ASSERTeq(cpu, cpu_check);
304+
ASSERT_EQ(cpu, cpu_check);
279305

280306
int numa_node_number = numa_node_of_cpu(cpu);
281307
printf("Got CPU: %d, got numa node: %d\n", cpu, numa_node_number);
@@ -297,7 +323,7 @@ TEST_P(testNumaOnEachCpu, checkModeLocal) {
297323
int ret = sched_setaffinity(0, sizeof(cpu_set_t), mask);
298324
CPU_FREE(mask);
299325

300-
UT_ASSERTeq(ret, 0);
326+
ASSERT_EQ(ret, 0);
301327

302328
umf_os_memory_provider_params_t os_memory_provider_params =
303329
UMF_OS_MEMORY_PROVIDER_PARAMS_TEST;
@@ -312,7 +338,7 @@ TEST_P(testNumaOnEachCpu, checkModeLocal) {
312338

313339
// Verify we're on the expected CPU
314340
int cpu_check = sched_getcpu();
315-
UT_ASSERTeq(cpu, cpu_check);
341+
ASSERT_EQ(cpu, cpu_check);
316342

317343
int numa_node_number = numa_node_of_cpu(cpu);
318344
printf("Got CPU: %d, got numa node: %d\n", cpu, numa_node_number);
@@ -391,7 +417,16 @@ TEST_F(testNuma, checkModeInterleave) {
391417
EXPECT_NODE_EQ((char *)ptr + page_size * i, numa_nodes[index]);
392418
}
393419

394-
bitmask *retrieved_nodemask = retrieve_nodemask(ptr);
420+
auto [fileLine, retrieved_nodemask] = retrieve_nodemask(ptr);
421+
if (fileLine != 0) {
422+
if (retrieved_nodemask == nullptr) {
423+
GTEST_FAIL() << "retrieved_nodemask is nullptr " << __FILE__ << ": "
424+
<< fileLine;
425+
} else {
426+
GTEST_FAIL() << "ret is not equal to 0 " << __FILE__ << ": "
427+
<< fileLine;
428+
}
429+
}
395430
int ret = numa_bitmask_equal(retrieved_nodemask, nodemask);
396431
numa_bitmask_free(retrieved_nodemask);
397432

0 commit comments

Comments
 (0)