Skip to content

Commit e0ee625

Browse files
alexbatashevalexbatashev
committed
Address some feedback
1 parent 5250fe2 commit e0ee625

File tree

5 files changed

+123
-123
lines changed

5 files changed

+123
-123
lines changed

‎xpti/CMakeLists.txt

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@ option(XPTI_ENABLE_WERROR OFF)
1313
if (XPTI_ENABLE_WERROR)
1414
if(MSVC)
1515
set(CMAKE_CXX_FLAGS "/WX ${CMAKE_CXX_FLAGS}")
16-
add_definitions(
17-
-wd4996 # Suppress 'function': was declared deprecated'
18-
)
1916
else()
2017
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror")
2118
endif()

‎xptifw/CMakeLists.txt

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,6 @@ option(XPTI_ENABLE_WERROR OFF)
1818
if (XPTI_ENABLE_WERROR)
1919
if(MSVC)
2020
set(CMAKE_CXX_FLAGS "/WX ${CMAKE_CXX_FLAGS}")
21-
add_definitions(
22-
-wd4996 # Suppress 'function': was declared deprecated'
23-
)
2421
else()
2522
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror")
2623
endif()

‎xptifw/src/xpti_trace_framework.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,8 @@ class Notifications {
727727
#endif
728728
}
729729

730+
void clear() { MCallbacksByStream.clear(); }
731+
730732
private:
731733
#ifdef XPTI_STATISTICS
732734
std::string stringify_trace_type(xpti_trace_point_type_t TraceType) {
@@ -807,6 +809,7 @@ class Framework {
807809
MUniversalIDs = 1;
808810
MTracepoints.clear();
809811
MStringTableRef.clear();
812+
MNotifier.clear();
810813
}
811814

812815
inline void setTraceEnabled(bool yesOrNo = true) { MTraceEnabled = yesOrNo; }

‎xptifw/unit_test/xpti_api_tests.cpp

Lines changed: 62 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,24 @@
1111
#include <vector>
1212

1313
static int func_callback_update = 0;
14+
static bool TPCB2Called = false;
1415

15-
TEST(xptiApiTest, xptiInitializeBadInput) {
16+
class xptiApiTest : public ::testing::Test {
17+
protected:
18+
void SetUp() override {
19+
TPCB2Called = false;
20+
func_callback_update = 0;
21+
}
22+
23+
void TearDown() override { xptiReset(); }
24+
};
25+
26+
TEST_F(xptiApiTest, xptiInitializeBadInput) {
1627
auto Result = xptiInitialize(nullptr, 0, 0, nullptr);
1728
EXPECT_EQ(Result, xpti::result_t::XPTI_RESULT_INVALIDARG);
1829
}
1930

20-
TEST(xptiApiTest, xptiRegisterStringBadInput) {
31+
TEST_F(xptiApiTest, xptiRegisterStringBadInput) {
2132
char *TStr;
2233

2334
auto ID = xptiRegisterString(nullptr, nullptr);
@@ -28,7 +39,7 @@ TEST(xptiApiTest, xptiRegisterStringBadInput) {
2839
EXPECT_EQ(ID, xpti::invalid_id);
2940
}
3041

31-
TEST(xptiApiTest, xptiRegisterStringGoodInput) {
42+
TEST_F(xptiApiTest, xptiRegisterStringGoodInput) {
3243
char *TStr = nullptr;
3344

3445
auto ID = xptiRegisterString("foo", &TStr);
@@ -37,14 +48,14 @@ TEST(xptiApiTest, xptiRegisterStringGoodInput) {
3748
EXPECT_STREQ("foo", TStr);
3849
}
3950

40-
TEST(xptiApiTest, xptiLookupStringBadInput) {
51+
TEST_F(xptiApiTest, xptiLookupStringBadInput) {
4152
const char *TStr;
4253
xptiReset();
4354
TStr = xptiLookupString(-1);
4455
EXPECT_EQ(TStr, nullptr);
4556
}
4657

47-
TEST(xptiApiTest, xptiLookupStringGoodInput) {
58+
TEST_F(xptiApiTest, xptiLookupStringGoodInput) {
4859
char *TStr = nullptr;
4960

5061
auto ID = xptiRegisterString("foo", &TStr);
@@ -58,7 +69,7 @@ TEST(xptiApiTest, xptiLookupStringGoodInput) {
5869
EXPECT_STREQ("foo", LookUpString);
5970
}
6071

61-
TEST(xptiApiTest, xptiRegisterPayloadGoodInput) {
72+
TEST_F(xptiApiTest, xptiRegisterPayloadGoodInput) {
6273
xpti::payload_t p("foo", "foo.cpp", 10, 0, (void *)(0xdeadbeefull));
6374

6475
auto ID = xptiRegisterPayload(&p);
@@ -67,14 +78,14 @@ TEST(xptiApiTest, xptiRegisterPayloadGoodInput) {
6778
EXPECT_EQ(p.uid.hash(), ID);
6879
}
6980

70-
TEST(xptiApiTest, xptiRegisterPayloadBadInput) {
81+
TEST_F(xptiApiTest, xptiRegisterPayloadBadInput) {
7182
xpti::payload_t p;
7283

7384
auto ID = xptiRegisterPayload(&p);
7485
EXPECT_EQ(ID, xpti::invalid_uid);
7586
}
7687

77-
TEST(xptiApiTest, xptiGetUniqueId) {
88+
TEST_F(xptiApiTest, xptiGetUniqueId) {
7889
std::set<uint64_t> IDs;
7990
for (int i = 0; i < 10; ++i) {
8091
auto ID = xptiGetUniqueId();
@@ -84,24 +95,24 @@ TEST(xptiApiTest, xptiGetUniqueId) {
8495
}
8596
}
8697

87-
TEST(xptiApiTest, xptiRegisterStreamBadInput) {
98+
TEST_F(xptiApiTest, xptiRegisterStreamBadInput) {
8899
auto ID = xptiRegisterStream(nullptr);
89100
EXPECT_EQ(ID, (uint8_t)xpti::invalid_id);
90101
}
91102

92-
TEST(xptiApiTest, xptiRegisterStreamGoodInput) {
103+
TEST_F(xptiApiTest, xptiRegisterStreamGoodInput) {
93104
auto ID = xptiRegisterStream("foo");
94105
EXPECT_NE(ID, xpti::invalid_id);
95106
auto NewID = xptiRegisterStream("foo");
96107
EXPECT_EQ(ID, NewID);
97108
}
98109

99-
TEST(xptiApiTest, xptiUnregisterStreamBadInput) {
110+
TEST_F(xptiApiTest, xptiUnregisterStreamBadInput) {
100111
auto Result = xptiUnregisterStream(nullptr);
101112
EXPECT_EQ(Result, xpti::result_t::XPTI_RESULT_INVALIDARG);
102113
}
103114

104-
TEST(xptiApiTest, xptiUnregisterStreamGoodInput) {
115+
TEST_F(xptiApiTest, xptiUnregisterStreamGoodInput) {
105116
auto ID = xptiRegisterStream("foo");
106117
EXPECT_NE(ID, xpti::invalid_id);
107118
auto Result = xptiUnregisterStream("NoSuchStream");
@@ -111,7 +122,7 @@ TEST(xptiApiTest, xptiUnregisterStreamGoodInput) {
111122
EXPECT_EQ(NewResult, xpti::result_t::XPTI_RESULT_NOTFOUND);
112123
}
113124

114-
TEST(xptiApiTest, xptiMakeEventBadInput) {
125+
TEST_F(xptiApiTest, xptiMakeEventBadInput) {
115126
xpti::payload_t P;
116127
auto Result =
117128
xptiMakeEvent(nullptr, &P, 0, (xpti::trace_activity_type_t)1, nullptr);
@@ -125,7 +136,7 @@ TEST(xptiApiTest, xptiMakeEventBadInput) {
125136
EXPECT_EQ(Result, nullptr);
126137
}
127138

128-
TEST(xptiApiTest, xptiMakeEventGoodInput) {
139+
TEST_F(xptiApiTest, xptiMakeEventGoodInput) {
129140
uint64_t instance;
130141
xpti::payload_t Payload("foo", "foo.cpp", 1, 0, (void *)13);
131142
auto Result = xptiMakeEvent("foo", &Payload, 0,
@@ -139,37 +150,37 @@ TEST(xptiApiTest, xptiMakeEventGoodInput) {
139150
EXPECT_EQ(instance, 2);
140151
}
141152

142-
TEST(xptiApiTest, xptiFindEventBadInput) {
153+
TEST_F(xptiApiTest, xptiFindEventBadInput) {
143154
auto Result = xptiFindEvent(0);
144155
EXPECT_EQ(Result, nullptr);
145156
Result = xptiFindEvent(1000000);
146157
EXPECT_EQ(Result, nullptr);
147158
}
148159

149-
TEST(xptiApiTest, xptiFindEventGoodInput) {
150-
uint64_t Instance;
160+
TEST_F(xptiApiTest, xptiFindEventGoodInput) {
161+
uint64_t Instance = 0;
151162
xpti::payload_t Payload("foo", "foo.cpp", 1, 0, (void *)13);
152163

153164
auto Result = xptiMakeEvent("foo", &Payload, 0,
154165
(xpti::trace_activity_type_t)1, &Instance);
155166
EXPECT_NE(Result, nullptr);
156-
EXPECT_GT(Instance, 1);
167+
EXPECT_EQ(Instance, 1);
157168
auto NewResult = xptiFindEvent(Result->unique_id);
158169
EXPECT_EQ(Result, NewResult);
159170
}
160171

161-
TEST(xptiApiTest, xptiQueryPayloadBadInput) {
172+
TEST_F(xptiApiTest, xptiQueryPayloadBadInput) {
162173
auto Result = xptiQueryPayload(nullptr);
163174
EXPECT_EQ(Result, nullptr);
164175
}
165176

166-
TEST(xptiApiTest, xptiQueryPayloadGoodInput) {
177+
TEST_F(xptiApiTest, xptiQueryPayloadGoodInput) {
167178
uint64_t instance;
168179
xpti::payload_t Payload("foo", "foo.cpp", 1, 0, (void *)13);
169180
auto Result = xptiMakeEvent("foo", &Payload, 0,
170181
(xpti::trace_activity_type_t)1, &instance);
171182
EXPECT_NE(Result, nullptr);
172-
EXPECT_GT(instance, 1);
183+
EXPECT_EQ(instance, 1);
173184
auto NewResult = xptiQueryPayload(Result);
174185
EXPECT_STREQ(Payload.name, NewResult->name);
175186
EXPECT_STREQ(Payload.source_file, NewResult->source_file);
@@ -179,7 +190,7 @@ TEST(xptiApiTest, xptiQueryPayloadGoodInput) {
179190
EXPECT_EQ(Payload.line_no, NewResult->line_no);
180191
}
181192

182-
TEST(xptiApiTest, xptiQueryPayloadByUIDGoodInput) {
193+
TEST_F(xptiApiTest, xptiQueryPayloadByUIDGoodInput) {
183194
xpti::payload_t p("foo", "foo.cpp", 10, 0, (void *)(0xdeadbeefull));
184195

185196
auto ID = xptiRegisterPayload(&p);
@@ -192,7 +203,7 @@ TEST(xptiApiTest, xptiQueryPayloadByUIDGoodInput) {
192203
EXPECT_EQ(p.uid.hash(), pp->uid.hash());
193204
}
194205

195-
TEST(xptiApiTest, xptiTraceEnabled) {
206+
TEST_F(xptiApiTest, xptiTraceEnabled) {
196207
// If no env is set, this should be false
197208
// The state is determined at app startup
198209
// XPTI_TRACE_ENABLE=1 or 0 and XPTI_FRAMEWORK_DISPATCHER=
@@ -201,35 +212,31 @@ TEST(xptiApiTest, xptiTraceEnabled) {
201212
EXPECT_EQ(Result, false);
202213
}
203214

204-
void trace_point_callback(uint16_t trace_type, xpti::trace_event_data_t *parent,
205-
xpti::trace_event_data_t *event, uint64_t instance,
206-
const void *user_data) {
207-
208-
if (user_data)
209-
(*static_cast<int *>(const_cast<void *>(user_data))) = 1;
210-
}
215+
void trace_point_callback(uint16_t /*trace_type*/,
216+
xpti::trace_event_data_t * /*parent*/,
217+
xpti::trace_event_data_t * /*event*/,
218+
uint64_t /*instance*/, const void * /*user_data*/) {}
211219

212-
void trace_point_callback2(uint16_t trace_type,
213-
xpti::trace_event_data_t *parent,
214-
xpti::trace_event_data_t *event, uint64_t instance,
215-
const void *user_data) {
216-
if (user_data)
217-
(*static_cast<int *>(const_cast<void *>(user_data))) = 1;
220+
void trace_point_callback2(uint16_t /*trace_type*/,
221+
xpti::trace_event_data_t * /*parent*/,
222+
xpti::trace_event_data_t * /*event*/,
223+
uint64_t /*instance*/, const void * /*user_data*/) {
224+
TPCB2Called = true;
218225
}
219226

220-
void fn_callback(uint16_t trace_type, xpti::trace_event_data_t *parent,
221-
xpti::trace_event_data_t *event, uint64_t instance,
222-
const void *user_data) {
227+
void fn_callback(uint16_t /*trace_type*/, xpti::trace_event_data_t * /*parent*/,
228+
xpti::trace_event_data_t * /*event*/, uint64_t /*instance*/,
229+
const void * /*user_data*/) {
223230
func_callback_update++;
224231
}
225232

226-
TEST(xptiApiTest, xptiRegisterCallbackBadInput) {
233+
TEST_F(xptiApiTest, xptiRegisterCallbackBadInput) {
227234
uint8_t StreamID = xptiRegisterStream("foo");
228235
auto Result = xptiRegisterCallback(StreamID, 1, nullptr);
229236
EXPECT_EQ(Result, xpti::result_t::XPTI_RESULT_INVALIDARG);
230237
}
231238

232-
TEST(xptiApiTest, xptiRegisterCallbackGoodInput) {
239+
TEST_F(xptiApiTest, xptiRegisterCallbackGoodInput) {
233240
uint64_t instance;
234241
xpti::payload_t Payload("foo", "foo.cpp", 1, 0, (void *)13);
235242

@@ -249,15 +256,13 @@ TEST(xptiApiTest, xptiRegisterCallbackGoodInput) {
249256
EXPECT_EQ(Result, xpti::result_t::XPTI_RESULT_SUCCESS);
250257
}
251258

252-
TEST(xptiApiTest, xptiUnregisterCallbackBadInput) {
259+
TEST_F(xptiApiTest, xptiUnregisterCallbackBadInput) {
253260
uint8_t StreamID = xptiRegisterStream("foo");
254261
auto Result = xptiUnregisterCallback(StreamID, 1, nullptr);
255262
EXPECT_EQ(Result, xpti::result_t::XPTI_RESULT_INVALIDARG);
256263
}
257264

258-
// TODO this test passes on Linux and fails on Windows.
259-
// Re-enable once it's sorted out.
260-
TEST(xptiApiTest, DISABLED_xptiUnregisterCallbackGoodInput) {
265+
TEST_F(xptiApiTest, xptiUnregisterCallbackGoodInput) {
261266
uint64_t instance;
262267
xpti::payload_t Payload("foo", "foo.cpp", 1, 0, (void *)13);
263268

@@ -278,7 +283,7 @@ TEST(xptiApiTest, DISABLED_xptiUnregisterCallbackGoodInput) {
278283
EXPECT_EQ(Result, xpti::result_t::XPTI_RESULT_UNDELETE);
279284
}
280285

281-
TEST(xptiApiTest, xptiNotifySubscribersBadInput) {
286+
TEST_F(xptiApiTest, xptiNotifySubscribersBadInput) {
282287
uint8_t StreamID = xptiRegisterStream("foo");
283288
auto Result =
284289
xptiNotifySubscribers(StreamID, 1, nullptr, nullptr, 0, nullptr);
@@ -292,7 +297,7 @@ TEST(xptiApiTest, xptiNotifySubscribersBadInput) {
292297
EXPECT_EQ(Result, xpti::result_t::XPTI_RESULT_INVALIDARG);
293298
}
294299

295-
TEST(xptiApiTest, xptiNotifySubscribersGoodInput) {
300+
TEST_F(xptiApiTest, xptiNotifySubscribersGoodInput) {
296301
uint64_t instance;
297302
xpti::payload_t Payload("foo", "foo.cpp", 1, 0, (void *)13);
298303

@@ -304,11 +309,16 @@ TEST(xptiApiTest, xptiNotifySubscribersGoodInput) {
304309
xptiForceSetTraceEnabled(true);
305310
int foo_return = 0;
306311
auto Result = xptiRegisterCallback(StreamID, 1, trace_point_callback2);
312+
EXPECT_EQ(Result, xpti::result_t::XPTI_RESULT_SUCCESS);
307313
Result = xptiNotifySubscribers(StreamID, 1, nullptr, Event, 0,
308314
(void *)(&foo_return));
309315
EXPECT_EQ(Result, xpti::result_t::XPTI_RESULT_SUCCESS);
310-
EXPECT_EQ(foo_return, 1);
316+
EXPECT_TRUE(TPCB2Called);
311317
int tmp = func_callback_update;
318+
Result = xptiRegisterCallback(
319+
StreamID, static_cast<uint16_t>(xpti::trace_point_type_t::function_begin),
320+
fn_callback);
321+
EXPECT_EQ(Result, xpti::result_t::XPTI_RESULT_SUCCESS);
312322
// We allow notification with parent and event being null, only for trace
313323
// point type of function_begin/end; This test update checks for that
314324
Result = xptiNotifySubscribers(
@@ -324,7 +334,7 @@ TEST(xptiApiTest, xptiNotifySubscribersGoodInput) {
324334
EXPECT_NE(tmp, func_callback_update);
325335
}
326336

327-
TEST(xptiApiTest, xptiAddMetadataBadInput) {
337+
TEST_F(xptiApiTest, xptiAddMetadataBadInput) {
328338
uint64_t instance;
329339
xpti::payload_t Payload("foo", "foo.cpp", 1, 0, (void *)13);
330340

@@ -342,7 +352,7 @@ TEST(xptiApiTest, xptiAddMetadataBadInput) {
342352
EXPECT_EQ(Result, xpti::result_t::XPTI_RESULT_INVALIDARG);
343353
}
344354

345-
TEST(xptiApiTest, xptiAddMetadataGoodInput) {
355+
TEST_F(xptiApiTest, xptiAddMetadataGoodInput) {
346356
uint64_t instance;
347357
xpti::payload_t Payload("foo", "foo.cpp", 1, 0, (void *)13);
348358

@@ -356,7 +366,7 @@ TEST(xptiApiTest, xptiAddMetadataGoodInput) {
356366
EXPECT_EQ(Result, xpti::result_t::XPTI_RESULT_DUPLICATE);
357367
}
358368

359-
TEST(xptiApiTest, xptiQueryMetadata) {
369+
TEST_F(xptiApiTest, xptiQueryMetadata) {
360370
uint64_t instance;
361371
xpti::payload_t Payload("foo", "foo.cpp", 1, 0, (void *)13);
362372

@@ -371,7 +381,7 @@ TEST(xptiApiTest, xptiQueryMetadata) {
371381
EXPECT_EQ(Result, xpti::result_t::XPTI_RESULT_SUCCESS);
372382

373383
char *ts;
374-
EXPECT_TRUE(md->size() > 1);
384+
EXPECT_EQ(md->size(), 1);
375385
auto ID = (*md)[xptiRegisterString("foo1", &ts)];
376386
auto str = xptiLookupString(ID);
377387
EXPECT_STREQ(str, "bar1");

0 commit comments

Comments
 (0)