Skip to content

Commit a9565d1

Browse files
committed
Remove redundant check of return value of check_for_gdb() in gdb api unit tests
check_for_gdb() could only return true since if the gdb invocation in its body failed a REQUIRE(...) in its body would fail. This changes check_for_gdb() the return type of check_for_gdb() to void and refactors its callees.
1 parent ac34e7e commit a9565d1

File tree

1 file changed

+67
-80
lines changed

1 file changed

+67
-80
lines changed

unit/memory-analyzer/gdb_api.cpp

Lines changed: 67 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ void compile_test_file()
3838
run("gcc", {"gcc", "-g", "-o", "test", "memory-analyzer/test.c"}) == 0);
3939
}
4040

41+
void check_for_gdb()
42+
{
43+
REQUIRE(run("gdb", {"gdb", "--version"}, "", "/dev/null", "/dev/null") == 0);
44+
}
45+
4146
class gdb_api_testt : public gdb_apit
4247
{
4348
explicit gdb_api_testt(const char *binary) : gdb_apit(binary)
@@ -49,6 +54,7 @@ class gdb_api_testt : public gdb_apit
4954

5055
void gdb_api_internals_test()
5156
{
57+
check_for_gdb();
5258
compile_test_file();
5359

5460
SECTION("parse gdb output record")
@@ -94,117 +100,98 @@ void gdb_api_internals_test()
94100
}
95101
}
96102

97-
bool check_for_gdb()
98-
{
99-
const bool has_gdb =
100-
run("gdb", {"gdb", "--version"}, "", "/dev/null", "/dev/null") == 0;
101-
102-
SECTION("check gdb is on the PATH")
103-
{
104-
REQUIRE(has_gdb);
105-
}
106-
return has_gdb;
107-
}
108-
109103
#endif
110104

111105
TEST_CASE("gdb api internals test", "[core][memory-analyzer]")
112106
{
113107
#ifdef RUN_GDB_API_TESTS
114-
if(check_for_gdb())
115-
{
116-
gdb_api_internals_test();
117-
}
108+
gdb_api_internals_test();
118109
#endif
119110
}
120111

121112
TEST_CASE("gdb api test", "[core][memory-analyzer]")
122113
{
123114
#ifdef RUN_GDB_API_TESTS
124-
if(check_for_gdb())
115+
check_for_gdb();
116+
compile_test_file();
117+
125118
{
126-
compile_test_file();
119+
gdb_apit gdb_api("test", true);
120+
gdb_api.create_gdb_process();
127121

122+
try
123+
{
124+
const bool r = gdb_api.run_gdb_to_breakpoint("checkpoint");
125+
REQUIRE(r);
126+
}
127+
catch(const gdb_interaction_exceptiont &e)
128128
{
129-
gdb_apit gdb_api("test", true);
130-
gdb_api.create_gdb_process();
129+
std::cerr << "warning: cannot fully unit test GDB API as program cannot "
130+
<< "be run with gdb\n";
131+
std::cerr << "warning: this may be due to not having the required "
132+
<< "permissions (e.g., to invoke ptrace() or to disable ASLR)"
133+
<< "\n";
134+
std::cerr << "gdb_interaction_exceptiont:" << '\n';
135+
std::cerr << e.what() << '\n';
136+
137+
std::ifstream file("gdb.txt");
138+
CHECK_RETURN(file.is_open());
139+
std::string line;
131140

132-
try
141+
std::cerr << "=== gdb log begin ===\n";
142+
143+
while(getline(file, line))
133144
{
134-
const bool r = gdb_api.run_gdb_to_breakpoint("checkpoint");
135-
REQUIRE(r);
145+
std::cerr << line << '\n';
136146
}
137-
catch(const gdb_interaction_exceptiont &e)
138-
{
139-
std::cerr
140-
<< "warning: cannot fully unit test GDB API as program cannot "
141-
<< "be run with gdb\n";
142-
std::cerr << "warning: this may be due to not having the required "
143-
<< "permissions (e.g., to invoke ptrace() or to disable ASLR)"
144-
<< "\n";
145-
std::cerr << "gdb_interaction_exceptiont:" << '\n';
146-
std::cerr << e.what() << '\n';
147147

148-
std::ifstream file("gdb.txt");
149-
CHECK_RETURN(file.is_open());
150-
std::string line;
148+
file.close();
149+
150+
std::cerr << "=== gdb log end ===\n";
151151

152-
std::cerr << "=== gdb log begin ===\n";
152+
return;
153+
}
154+
}
153155

154-
while(getline(file, line))
155-
{
156-
std::cerr << line << '\n';
157-
}
156+
gdb_apit gdb_api("test");
157+
gdb_api.create_gdb_process();
158158

159-
file.close();
159+
SECTION("breakpoint is hit")
160+
{
161+
const bool r = gdb_api.run_gdb_to_breakpoint("checkpoint");
162+
REQUIRE(r);
163+
}
160164

161-
std::cerr << "=== gdb log end ===\n";
165+
SECTION("breakpoint is not hit")
166+
{
167+
const bool r = gdb_api.run_gdb_to_breakpoint("checkpoint2");
168+
REQUIRE(!r);
169+
}
162170

163-
return;
164-
}
165-
}
171+
SECTION("breakpoint does not exist")
172+
{
173+
REQUIRE_THROWS_AS(
174+
gdb_api.run_gdb_to_breakpoint("checkpoint3"), gdb_interaction_exceptiont);
175+
}
166176

167-
gdb_apit gdb_api("test");
168-
gdb_api.create_gdb_process();
177+
SECTION("query memory")
178+
{
179+
const bool r = gdb_api.run_gdb_to_breakpoint("checkpoint");
180+
REQUIRE(r);
169181

170-
SECTION("breakpoint is hit")
171-
{
172-
const bool r = gdb_api.run_gdb_to_breakpoint("checkpoint");
173-
REQUIRE(r);
174-
}
182+
REQUIRE(gdb_api.get_value("x") == "8");
183+
REQUIRE(gdb_api.get_value("s") == "abc");
175184

176-
SECTION("breakpoint is not hit")
177-
{
178-
const bool r = gdb_api.run_gdb_to_breakpoint("checkpoint2");
179-
REQUIRE(!r);
180-
}
185+
const std::regex regex(R"(0x[1-9a-f][0-9a-f]*)");
181186

182-
SECTION("breakpoint does not exist")
183187
{
184-
REQUIRE_THROWS_AS(
185-
gdb_api.run_gdb_to_breakpoint("checkpoint3"),
186-
gdb_interaction_exceptiont);
188+
std::string address = gdb_api.get_memory("p");
189+
REQUIRE(std::regex_match(address, regex));
187190
}
188191

189-
SECTION("query memory")
190192
{
191-
const bool r = gdb_api.run_gdb_to_breakpoint("checkpoint");
192-
REQUIRE(r);
193-
194-
REQUIRE(gdb_api.get_value("x") == "8");
195-
REQUIRE(gdb_api.get_value("s") == "abc");
196-
197-
const std::regex regex(R"(0x[1-9a-f][0-9a-f]*)");
198-
199-
{
200-
std::string address = gdb_api.get_memory("p");
201-
REQUIRE(std::regex_match(address, regex));
202-
}
203-
204-
{
205-
std::string address = gdb_api.get_memory("vp");
206-
REQUIRE(std::regex_match(address, regex));
207-
}
193+
std::string address = gdb_api.get_memory("vp");
194+
REQUIRE(std::regex_match(address, regex));
208195
}
209196
}
210197
#endif

0 commit comments

Comments
 (0)