Skip to content

Commit f27254c

Browse files
author
Razvan Becheriu
committed
[#3315] use enum SpawnMode instead of bool
1 parent a070713 commit f27254c

File tree

9 files changed

+48
-44
lines changed

9 files changed

+48
-44
lines changed

src/hooks/dhcp/mysql_cb/mysql_cb_callouts.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ int load(LibraryHandle& /* handle */) {
6868
///
6969
/// @param handle callout handle passed to the callout.
7070
/// @return 0 on success, 1 otherwise.
71-
int dhcp4_srv_configured(CalloutHandle& handle) {
71+
int dhcp4_srv_configured(CalloutHandle& /* handle */) {
7272
isc::dhcp::MySqlConfigBackendImpl::setIOService(IOServicePtr(new IOService()));
7373
IOServiceMgr::instance().registerIOService(isc::dhcp::MySqlConfigBackendImpl::getIOService());
7474
return (0);
@@ -80,7 +80,7 @@ int dhcp4_srv_configured(CalloutHandle& handle) {
8080
///
8181
/// @param handle callout handle passed to the callout.
8282
/// @return 0 on success, 1 otherwise.
83-
int dhcp6_srv_configured(CalloutHandle& handle) {
83+
int dhcp6_srv_configured(CalloutHandle& /* handle */) {
8484
isc::dhcp::MySqlConfigBackendImpl::setIOService(IOServicePtr(new IOService()));
8585
IOServiceMgr::instance().registerIOService(isc::dhcp::MySqlConfigBackendImpl::getIOService());
8686
return (0);

src/hooks/dhcp/pgsql_cb/pgsql_cb_callouts.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ int load(LibraryHandle& /* handle */) {
6868
///
6969
/// @param handle callout handle passed to the callout.
7070
/// @return 0 on success, 1 otherwise.
71-
int dhcp4_srv_configured(CalloutHandle& handle) {
71+
int dhcp4_srv_configured(CalloutHandle& /* handle */) {
7272
isc::dhcp::PgSqlConfigBackendImpl::setIOService(IOServicePtr(new IOService()));
7373
IOServiceMgr::instance().registerIOService(isc::dhcp::PgSqlConfigBackendImpl::getIOService());
7474
return (0);
@@ -80,7 +80,7 @@ int dhcp4_srv_configured(CalloutHandle& handle) {
8080
///
8181
/// @param handle callout handle passed to the callout.
8282
/// @return 0 on success, 1 otherwise.
83-
int dhcp6_srv_configured(CalloutHandle& handle) {
83+
int dhcp6_srv_configured(CalloutHandle& /* handle */) {
8484
isc::dhcp::PgSqlConfigBackendImpl::setIOService(IOServicePtr(new IOService()));
8585
IOServiceMgr::instance().registerIOService(isc::dhcp::PgSqlConfigBackendImpl::getIOService());
8686
return (0);

src/hooks/dhcp/run_script/run_script.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ RunScriptImpl::configure(LibraryHandle& handle) {
3131
isc_throw(InvalidParameter, "The 'name' parameter must be a string");
3232
}
3333
try {
34-
ProcessSpawn process(false, name->stringValue());
34+
ProcessSpawn process(ProcessSpawn::ASYNC, name->stringValue());
3535
} catch (const isc::Exception& ex) {
3636
isc_throw(InvalidParameter, "Invalid 'name' parameter: " << ex.what());
3737
}
@@ -47,7 +47,7 @@ RunScriptImpl::configure(LibraryHandle& handle) {
4747

4848
void
4949
RunScriptImpl::runScript(const ProcessArgs& args, const ProcessEnvVars& vars) {
50-
ProcessSpawn process(false, name_, args, vars);
50+
ProcessSpawn process(ProcessSpawn::ASYNC, name_, args, vars);
5151
process.spawn(true);
5252
}
5353

src/lib/asiolink/process_spawn.cc

+9-10
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,13 @@ class ProcessSpawnImpl : boost::noncopyable {
7777

7878
/// @brief Constructor.
7979
///
80-
/// @param sync enables synchronous mode (spawning thread waits on
81-
/// child to complete if true)
80+
/// @param mode specifies synchronous or asynchronous mode.
8281
/// @param executable A full path to the program to be executed.
8382
/// @param args Arguments for the program to be executed.
8483
/// @param vars Environment variables for the program to be executed.
8584
/// @param inherit_env whether the spawned process will inherit the
8685
/// environment before adding 'vars' on top.
87-
ProcessSpawnImpl(const bool sync,
86+
ProcessSpawnImpl(const ProcessSpawn::SpawnMode mode,
8887
const std::string& executable,
8988
const ProcessArgs& args,
9089
const ProcessEnvVars& vars,
@@ -217,7 +216,7 @@ class ProcessSpawnImpl : boost::noncopyable {
217216

218217
/// @brief Whether the process is waited immediately after spawning
219218
/// (synchronous) or not (asynchronous).
220-
bool sync_;
219+
ProcessSpawn::SpawnMode mode_;
221220

222221
/// @brief Path to an executable.
223222
std::string executable_;
@@ -248,12 +247,12 @@ void ProcessSpawnImpl::IOSignalSetInitializer::initIOSignalSet(IOServicePtr io_s
248247
static IOSignalSetInitializer init(io_service);
249248
}
250249

251-
ProcessSpawnImpl::ProcessSpawnImpl(const bool sync,
250+
ProcessSpawnImpl::ProcessSpawnImpl(const ProcessSpawn::SpawnMode mode,
252251
const std::string& executable,
253252
const ProcessArgs& args,
254253
const ProcessEnvVars& vars,
255254
const bool inherit_env)
256-
: sync_(sync), executable_(executable), args_(new char*[args.size() + 2]),
255+
: mode_(mode), executable_(executable), args_(new char*[args.size() + 2]),
257256
store_(false) {
258257

259258
// Size of vars except the trailing null
@@ -329,7 +328,7 @@ ProcessSpawnImpl::getCommandLine() const {
329328
pid_t
330329
ProcessSpawnImpl::spawn(bool dismiss) {
331330
lock_guard<std::mutex> lk(mutex_);
332-
if (!sync_) {
331+
if (mode_ == ProcessSpawn::ASYNC) {
333332
ProcessSpawnImpl::IOSignalSetInitializer::initIOSignalSet(ProcessSpawn::getIOService());
334333
}
335334
// Create the child
@@ -355,7 +354,7 @@ ProcessSpawnImpl::spawn(bool dismiss) {
355354
process_collection_[this].insert(std::pair<pid_t, ProcessStatePtr>(pid, ProcessStatePtr(new ProcessState())));
356355
}
357356

358-
if (sync_) {
357+
if (mode_ == ProcessSpawn::SYNC) {
359358
waitForProcess(SIGCHLD, pid, /* sync = */ true);
360359
}
361360

@@ -476,12 +475,12 @@ ProcessSpawnImpl::clearState(const pid_t pid) {
476475

477476
IOServicePtr ProcessSpawn::io_service_;
478477

479-
ProcessSpawn::ProcessSpawn(const bool sync,
478+
ProcessSpawn::ProcessSpawn(const SpawnMode mode,
480479
const std::string& executable,
481480
const ProcessArgs& args,
482481
const ProcessEnvVars& vars,
483482
const bool inherit_env /* = false */)
484-
: impl_(new ProcessSpawnImpl(sync, executable, args, vars, inherit_env)) {
483+
: impl_(new ProcessSpawnImpl(mode, executable, args, vars, inherit_env)) {
485484
}
486485

487486
std::string

src/lib/asiolink/process_spawn.h

+8-3
Original file line numberDiff line numberDiff line change
@@ -61,16 +61,21 @@ typedef std::vector<std::string> ProcessEnvVars;
6161
class ProcessSpawn : boost::noncopyable {
6262
public:
6363

64+
/// @brief The spawn type.
65+
enum SpawnMode {
66+
ASYNC, // thread continues without waiting for the child to finish.
67+
SYNC // thread waits for the child to finish.
68+
};
69+
6470
/// @brief Constructor.
6571
///
66-
/// @param sync enables synchronous mode (spawning thread waits on
67-
/// child to complete if true)
72+
/// @param mode specifies synchronous or asynchronous mode.
6873
/// @param executable A full path to the program to be executed.
6974
/// @param args Arguments for the program to be executed.
7075
/// @param vars Environment variables for the program to be executed.
7176
/// @param inherit_env whether the spawned process will inherit the
7277
/// environment before adding 'vars' on top.
73-
ProcessSpawn(bool sync,
78+
ProcessSpawn(const SpawnMode mode,
7479
const std::string& executable,
7580
const ProcessArgs& args = ProcessArgs(),
7681
const ProcessEnvVars& vars = ProcessEnvVars(),

src/lib/asiolink/tests/process_spawn_unittest.cc

+22-22
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ TEST_F(ProcessSpawnTest, spawnWithArgs) {
110110
args.push_back("-e");
111111
args.push_back("64");
112112

113-
ProcessSpawn process(false, TEST_SCRIPT_SH, args);
113+
ProcessSpawn process(ProcessSpawn::ASYNC, TEST_SCRIPT_SH, args);
114114
pid_t pid = 0;
115115
ASSERT_NO_THROW(pid = process.spawn());
116116

@@ -144,7 +144,7 @@ TEST_F(ProcessSpawnTest, spawnWithArgsAndEnvVars) {
144144
args.push_back("TEST_VARIABLE_VALUE");
145145
vars.push_back("TEST_VARIABLE_NAME=TEST_VARIABLE_VALUE");
146146

147-
ProcessSpawn process(false, TEST_SCRIPT_SH, args, vars);
147+
ProcessSpawn process(ProcessSpawn::ASYNC, TEST_SCRIPT_SH, args, vars);
148148
pid_t pid = 0;
149149
ASSERT_NO_THROW(pid = process.spawn());
150150

@@ -176,7 +176,7 @@ TEST_F(ProcessSpawnTest, spawnTwoProcesses) {
176176
vector<string> args;
177177
args.push_back("-p");
178178

179-
ProcessSpawn process(false, TEST_SCRIPT_SH, args);
179+
ProcessSpawn process(ProcessSpawn::ASYNC, TEST_SCRIPT_SH, args);
180180
pid_t pid1 = 0;
181181
ASSERT_NO_THROW(pid1 = process.spawn());
182182

@@ -232,7 +232,7 @@ TEST_F(ProcessSpawnTest, spawnTwoProcesses) {
232232
// This test verifies that the external application can be ran without
233233
// arguments and that the exit code is gathered.
234234
TEST_F(ProcessSpawnTest, spawnNoArgs) {
235-
ProcessSpawn process(false, TEST_SCRIPT_SH);
235+
ProcessSpawn process(ProcessSpawn::ASYNC, TEST_SCRIPT_SH);
236236
pid_t pid = 0;
237237
ASSERT_NO_THROW(pid = process.spawn());
238238

@@ -281,14 +281,14 @@ TEST_F(ProcessSpawnTest, spawnNoArgs) {
281281
// application can't be executed.
282282
TEST_F(ProcessSpawnTest, invalidExecutable) {
283283
std::string expected = "File not found: foo";
284-
ASSERT_THROW_MSG(ProcessSpawn process(false, "foo"),
284+
ASSERT_THROW_MSG(ProcessSpawn process(ProcessSpawn::ASYNC, "foo"),
285285
ProcessSpawnError, expected);
286286

287287
std::string name = INVALID_TEST_SCRIPT_SH;
288288

289289
expected = "File not executable: ";
290290
expected += name;
291-
ASSERT_THROW_MSG(ProcessSpawn process(false, name),
291+
ASSERT_THROW_MSG(ProcessSpawn process(ProcessSpawn::ASYNC, name),
292292
ProcessSpawnError, expected);
293293
}
294294

@@ -302,15 +302,15 @@ TEST_F(ProcessSpawnTest, getCommandLine) {
302302
args.push_back("-y");
303303
args.push_back("foo");
304304
args.push_back("bar");
305-
ProcessSpawn process(false, TEST_SCRIPT_SH, args);
305+
ProcessSpawn process(ProcessSpawn::ASYNC, TEST_SCRIPT_SH, args);
306306
std::string expected = TEST_SCRIPT_SH;
307307
expected += " -x -y foo bar";
308308
EXPECT_EQ(expected, process.getCommandLine());
309309
}
310310

311311
{
312312
// Case 2: no arguments.
313-
ProcessSpawn process(false, TEST_SCRIPT_SH);
313+
ProcessSpawn process(ProcessSpawn::ASYNC, TEST_SCRIPT_SH);
314314
EXPECT_EQ(TEST_SCRIPT_SH, process.getCommandLine());
315315
}
316316
}
@@ -324,7 +324,7 @@ TEST_F(ProcessSpawnTest, isRunning) {
324324
args.push_back("-s");
325325
args.push_back("10");
326326

327-
ProcessSpawn process(false, TEST_SCRIPT_SH, args);
327+
ProcessSpawn process(ProcessSpawn::ASYNC, TEST_SCRIPT_SH, args);
328328
pid_t pid = 0;
329329
ASSERT_NO_THROW(pid = process.spawn());
330330

@@ -358,7 +358,7 @@ TEST_F(ProcessSpawnTest, inheritEnv) {
358358

359359
ProcessEnvVars vars{"VAR=value"};
360360

361-
ProcessSpawn process(false, TEST_SCRIPT_SH, args, vars,
361+
ProcessSpawn process(ProcessSpawn::ASYNC, TEST_SCRIPT_SH, args, vars,
362362
/* inherit_env = */ true);
363363
pid_t pid = 0;
364364
ASSERT_NO_THROW(pid = process.spawn());
@@ -392,7 +392,7 @@ TEST_F(ProcessSpawnTest, inheritEnvWithParentVar) {
392392

393393
ProcessEnvVars vars{"VAR=value"};
394394

395-
ProcessSpawn process(false, TEST_SCRIPT_SH, args, vars,
395+
ProcessSpawn process(ProcessSpawn::ASYNC, TEST_SCRIPT_SH, args, vars,
396396
/* inherit_env = */ true);
397397
pid_t pid = 0;
398398
ASSERT_NO_THROW(pid = process.spawn());
@@ -424,7 +424,7 @@ TEST_F(ProcessSpawnTest, spawnWithArgsSync) {
424424
args.push_back("-e");
425425
args.push_back("64");
426426

427-
ProcessSpawn process(true, TEST_SCRIPT_SH, args);
427+
ProcessSpawn process(ProcessSpawn::SYNC, TEST_SCRIPT_SH, args);
428428
pid_t pid = 0;
429429
ASSERT_NO_THROW(pid = process.spawn());
430430

@@ -442,7 +442,7 @@ TEST_F(ProcessSpawnTest, spawnWithArgsAndEnvVarsSync) {
442442
args.push_back("TEST_VARIABLE_VALUE");
443443
vars.push_back("TEST_VARIABLE_NAME=TEST_VARIABLE_VALUE");
444444

445-
ProcessSpawn process(true, TEST_SCRIPT_SH, args, vars);
445+
ProcessSpawn process(ProcessSpawn::SYNC, TEST_SCRIPT_SH, args, vars);
446446
pid_t pid = 0;
447447
ASSERT_NO_THROW(pid = process.spawn());
448448

@@ -458,7 +458,7 @@ TEST_F(ProcessSpawnTest, spawnTwoProcessesSync) {
458458
vector<string> args;
459459
args.push_back("-p");
460460

461-
ProcessSpawn process(true, TEST_SCRIPT_SH, args);
461+
ProcessSpawn process(ProcessSpawn::SYNC, TEST_SCRIPT_SH, args);
462462
pid_t pid1 = 0;
463463
ASSERT_NO_THROW(pid1 = process.spawn());
464464

@@ -481,7 +481,7 @@ TEST_F(ProcessSpawnTest, spawnTwoProcessesSync) {
481481
// This test verifies that the external application can be ran synchronously
482482
// without arguments and that the exit code is gathered.
483483
TEST_F(ProcessSpawnTest, spawnNoArgsSync) {
484-
ProcessSpawn process(true, TEST_SCRIPT_SH);
484+
ProcessSpawn process(ProcessSpawn::SYNC, TEST_SCRIPT_SH);
485485
pid_t pid = 0;
486486
ASSERT_NO_THROW(pid = process.spawn());
487487

@@ -497,14 +497,14 @@ TEST_F(ProcessSpawnTest, spawnNoArgsSync) {
497497
// application can't be executed synchronously.
498498
TEST_F(ProcessSpawnTest, invalidExecutableSync) {
499499
std::string expected = "File not found: foo";
500-
ASSERT_THROW_MSG(ProcessSpawn process(true, "foo"),
500+
ASSERT_THROW_MSG(ProcessSpawn process(ProcessSpawn::SYNC, "foo"),
501501
ProcessSpawnError, expected);
502502

503503
std::string name = INVALID_TEST_SCRIPT_SH;
504504

505505
expected = "File not executable: ";
506506
expected += name;
507-
ASSERT_THROW_MSG(ProcessSpawn process(true, name),
507+
ASSERT_THROW_MSG(ProcessSpawn process(ProcessSpawn::SYNC, name),
508508
ProcessSpawnError, expected);
509509
}
510510

@@ -518,23 +518,23 @@ TEST_F(ProcessSpawnTest, getCommandLineSync) {
518518
args.push_back("-y");
519519
args.push_back("foo");
520520
args.push_back("bar");
521-
ProcessSpawn process(true, TEST_SCRIPT_SH, args);
521+
ProcessSpawn process(ProcessSpawn::SYNC, TEST_SCRIPT_SH, args);
522522
std::string expected = TEST_SCRIPT_SH;
523523
expected += " -x -y foo bar";
524524
EXPECT_EQ(expected, process.getCommandLine());
525525
}
526526

527527
{
528528
// Case 2: no arguments.
529-
ProcessSpawn process(true, TEST_SCRIPT_SH);
529+
ProcessSpawn process(ProcessSpawn::SYNC, TEST_SCRIPT_SH);
530530
EXPECT_EQ(TEST_SCRIPT_SH, process.getCommandLine());
531531
}
532532
}
533533

534534
// This test verifies that the synchronous process reports as not running after
535535
// it was spawned.
536536
TEST_F(ProcessSpawnTest, isRunningSync) {
537-
ProcessSpawn process(true, TEST_SCRIPT_SH);
537+
ProcessSpawn process(ProcessSpawn::SYNC, TEST_SCRIPT_SH);
538538
pid_t pid = 0;
539539
ASSERT_NO_THROW(pid = process.spawn());
540540

@@ -549,7 +549,7 @@ TEST_F(ProcessSpawnTest, inheritEnvSync) {
549549

550550
ProcessEnvVars vars{"VAR=value"};
551551

552-
ProcessSpawn process(true, TEST_SCRIPT_SH, args, vars,
552+
ProcessSpawn process(ProcessSpawn::SYNC, TEST_SCRIPT_SH, args, vars,
553553
/* inherit_env = */ true);
554554
pid_t pid = 0;
555555
ASSERT_NO_THROW(pid = process.spawn());
@@ -567,7 +567,7 @@ TEST_F(ProcessSpawnTest, inheritEnvWithParentVarSync) {
567567

568568
ProcessEnvVars vars{"VAR=value"};
569569

570-
ProcessSpawn process(true, TEST_SCRIPT_SH, args, vars,
570+
ProcessSpawn process(ProcessSpawn::SYNC, TEST_SCRIPT_SH, args, vars,
571571
/* inherit_env = */ true);
572572
pid_t pid = 0;
573573
ASSERT_NO_THROW(pid = process.spawn());

src/lib/dhcpsrv/memfile_lease_mgr.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ LFCSetup::setup(const uint32_t lfc_interval,
209209
args.push_back("ignored-path");
210210

211211
// Create the process (do not start it yet).
212-
process_.reset(new ProcessSpawn(false, executable, args));
212+
process_.reset(new ProcessSpawn(ProcessSpawn::ASYNC, executable, args));
213213

214214
// If we've been told to run it once now, invoke the callback directly.
215215
if (run_once_now) {

src/lib/mysql/mysql_connection.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ MySqlConnection::initializeSchema(const ParameterMap& parameters) {
434434
kea_admin_parameters.insert(kea_admin_parameters.begin(), "db-init");
435435

436436
// Run.
437-
ProcessSpawn kea_admin(true, KEA_ADMIN_, kea_admin_parameters, vars,
437+
ProcessSpawn kea_admin(ProcessSpawn::SYNC, KEA_ADMIN_, kea_admin_parameters, vars,
438438
/* inherit_env = */ true);
439439
DB_LOG_INFO(MYSQL_INITIALIZE_SCHEMA).arg(kea_admin.getCommandLine());
440440
pid_t const pid(kea_admin.spawn());

src/lib/pgsql/pgsql_connection.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ PgSqlConnection::initializeSchema(const ParameterMap& parameters) {
237237
kea_admin_parameters.insert(kea_admin_parameters.begin(), "db-init");
238238

239239
// Run.
240-
ProcessSpawn kea_admin(true, KEA_ADMIN_, kea_admin_parameters, vars,
240+
ProcessSpawn kea_admin(ProcessSpawn::SYNC, KEA_ADMIN_, kea_admin_parameters, vars,
241241
/* inherit_env = */ true);
242242
DB_LOG_INFO(PGSQL_INITIALIZE_SCHEMA).arg(kea_admin.getCommandLine());
243243
pid_t const pid(kea_admin.spawn());

0 commit comments

Comments
 (0)