Skip to content

Commit efeccd5

Browse files
rdementirdementi
authored
Merge pull request #248 from intel-innersource/agorneanu/242-drop-strncmp-argument-parsing-in-pcm-utils
Refactor argument parsing in pcm utilities
2 parents 306614e + ad9a193 commit efeccd5

13 files changed

+268
-435
lines changed

‎src/pcm-core.cpp

Lines changed: 22 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ extern "C" {
110110
}
111111
}
112112

113-
void print_usage(const string progname)
113+
void print_usage(const string & progname)
114114
{
115115
cout << "\n Usage: \n " << progname
116116
<< " --help | [delay] [options] [-- external_program [external_program_options]]\n";
@@ -298,6 +298,8 @@ int main(int argc, char * argv[])
298298
{
299299
argv++;
300300
argc--;
301+
string arg_value;
302+
301303
if (check_argument_equals(*argv, {"--help", "-h", "/h"}))
302304
{
303305
print_usage(program);
@@ -308,43 +310,36 @@ int main(int argc, char * argv[])
308310
// handled in check_and_set_silent
309311
continue;
310312
}
311-
else if (strncmp(*argv, "-csv",4) == 0 ||
312-
strncmp(*argv, "/csv",4) == 0)
313+
else if (check_argument_equals(*argv, {"-csv", "/csv"}))
313314
{
314315
csv = true;
315-
string cmd = string(*argv);
316-
size_t found = cmd.find('=',4);
317-
if (found != string::npos) {
318-
string filename = cmd.substr(found+1);
319-
if (!filename.empty()) {
320-
m->setOutput(filename);
321-
}
316+
}
317+
else if (extract_argument_value(*argv, {"-csv", "/csv"}, arg_value))
318+
{
319+
csv = true;
320+
if (!arg_value.empty()) {
321+
m->setOutput(arg_value);
322322
}
323323
continue;
324324
}
325-
else
326-
if (mainLoop.parseArg(*argv))
325+
else if (mainLoop.parseArg(*argv))
327326
{
328327
continue;
329328
}
330-
else if (strncmp(*argv, "-c",2) == 0 ||
331-
strncmp(*argv, "/c",2) == 0)
329+
else if (check_argument_equals(*argv, {"-c", "/c"}))
332330
{
333331
cout << m->getCPUFamilyModelString() << "\n";
334332
exit(EXIT_SUCCESS);
335333
}
336-
else if (strncmp(*argv, "-txn",4) == 0 ||
337-
strncmp(*argv, "/txn",4) == 0)
334+
else if (check_argument_equals(*argv, {"-txn", "/txn"}))
338335
{
339336
argv++;
340337
argc--;
341338
txn_rate = strtoull(*argv,NULL,10);
342339
cout << "txn_rate set to " << txn_rate << "\n";
343340
continue;
344341
}
345-
if (strncmp(*argv, "--yescores", 10) == 0 ||
346-
strncmp(*argv, "-yc", 3) == 0 ||
347-
strncmp(*argv, "/yc", 3) == 0)
342+
else if (check_argument_equals(*argv, {"--yescores", "-yc", "/yc"}))
348343
{
349344
argv++;
350345
argc--;
@@ -379,10 +374,11 @@ int main(int argc, char * argv[])
379374
}
380375
continue;
381376
}
382-
else if (strncmp(*argv, "-e",2) == 0)
377+
else if (check_argument_equals(*argv, {"-e"}))
383378
{
384379
argv++;
385380
argc--;
381+
386382
if(cur_event >= conf.nGPCounters) {
387383
cerr << "At most " << conf.nGPCounters << " events are allowed\n";
388384
exit(EXIT_FAILURE);
@@ -393,15 +389,13 @@ int main(int argc, char * argv[])
393389
} catch (...) {
394390
exit(EXIT_FAILURE);
395391
}
396-
397392
continue;
398393
}
399-
else
400-
if (CheckAndForceRTMAbortMode(*argv, m))
401-
{
402-
continue;
403-
}
404-
else if (strncmp(*argv, "--", 2) == 0)
394+
else if (CheckAndForceRTMAbortMode(*argv, m))
395+
{
396+
continue;
397+
}
398+
else if (check_argument_equals(*argv, {"--"}))
405399
{
406400
argv++;
407401
sysCmd = *argv;
@@ -410,18 +404,7 @@ int main(int argc, char * argv[])
410404
}
411405
else
412406
{
413-
// any other options positional that is a floating point number is treated as <delay>,
414-
// while the other options are ignored with a warning issues to stderr
415-
double delay_input = 0.0;
416-
std::istringstream is_str_stream(*argv);
417-
is_str_stream >> noskipws >> delay_input;
418-
if(is_str_stream.eof() && !is_str_stream.fail()) {
419-
delay = delay_input;
420-
} else {
421-
cerr << "WARNING: unknown command-line option: \"" << *argv << "\". Ignoring it.\n";
422-
print_usage(program);
423-
exit(EXIT_FAILURE);
424-
}
407+
delay = parse_delay(*argv, program, (print_usage_func)print_usage);
425408
continue;
426409
}
427410
} while(argc > 1); // end of command line parsing loop

‎src/pcm-iio.cpp

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,24 +1202,8 @@ int main(int argc, char * argv[])
12021202
continue;
12031203
}
12041204
else {
1205-
// any other options positional that is a floating point number is treated as <delay>,
1206-
// while the other options are ignored with a warning issues to stderr
1207-
double delay_input = 0.0;
1208-
istringstream is_str_stream(*argv);
1209-
is_str_stream >> noskipws >> delay_input;
1210-
if (is_str_stream.eof() && !is_str_stream.fail()) {
1211-
if (delay_input < 0) {
1212-
cerr << "Invalid delay specified: \"" << *argv << "\". Delay should be positive.\n";
1213-
print_usage(program);
1214-
exit(EXIT_FAILURE);
1215-
}
1216-
delay = delay_input;
1217-
}
1218-
else {
1219-
cerr << "WARNING: unknown command-line option: \"" << *argv << "\". Ignoring it.\n";
1220-
print_usage(program);
1221-
exit(EXIT_FAILURE);
1222-
}
1205+
delay = parse_delay(*argv, program, (print_usage_func)print_usage);
1206+
continue;
12231207
}
12241208
}
12251209

‎src/pcm-latency.cpp

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -491,20 +491,13 @@ int main(int argc, char * argv[])
491491
{
492492
continue;
493493
}
494-
else if (strncmp(*argv, "--PMM",6) == 0 || strncmp(*argv, "-pmm", 5) == 0)
494+
else if (check_argument_equals(*argv, {"--PMM", "-pmm"}))
495495
{
496-
argv++;
497-
argc--;
498496
enable_pmm = true;
499497
continue;
500498
}
501-
502-
else if (strncmp(*argv, "--verbose", 9) == 0 ||
503-
strncmp(*argv, "-v", 2) == 0 ||
504-
strncmp(*argv, "/v", 2) == 0)
499+
else if (check_argument_equals(*argv, {"--verbose", "-v", "/v"}))
505500
{
506-
argv++;
507-
argc--;
508501
enable_verbose = true;
509502
continue;
510503
}

‎src/pcm-memory.cpp

Lines changed: 44 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ bool anyPmem(const ServerUncoreMemoryMetrics & metrics)
7070

7171
bool skipInactiveChannels = true;
7272

73-
void print_help(const string prog_name)
73+
void print_help(const string & prog_name)
7474
{
7575
cout << "\n Usage: \n " << prog_name
7676
<< " --help | [delay] [options] [-- external_program [external_program_options]]\n";
@@ -1002,6 +1002,8 @@ int main(int argc, char * argv[])
10021002
{
10031003
argv++;
10041004
argc--;
1005+
string arg_value;
1006+
10051007
if (check_argument_equals(*argv, {"--help", "-h", "/h"}))
10061008
{
10071009
print_help(program);
@@ -1012,117 +1014,101 @@ int main(int argc, char * argv[])
10121014
// handled in check_and_set_silent
10131015
continue;
10141016
}
1015-
else if (strncmp(*argv, "-csv",4) == 0 ||
1016-
strncmp(*argv, "/csv",4) == 0)
1017+
else if (check_argument_equals(*argv, {"-csv", "/csv"}))
1018+
{
1019+
csv = csvheader = true;
1020+
}
1021+
else if (extract_argument_value(*argv, {"-csv", "/csv"}, arg_value))
10171022
{
10181023
csv = true;
10191024
csvheader = true;
1020-
string cmd = string(*argv);
1021-
size_t found = cmd.find('=',4);
1022-
if (found != string::npos) {
1023-
string filename = cmd.substr(found+1);
1024-
if (!filename.empty()) {
1025-
m->setOutput(filename);
1026-
}
1025+
if (!arg_value.empty()) {
1026+
m->setOutput(arg_value);
10271027
}
10281028
continue;
10291029
}
10301030
else if (mainLoop.parseArg(*argv))
10311031
{
10321032
continue;
10331033
}
1034-
else if (strncmp(*argv, "-columns", 8) == 0 ||
1035-
strncmp(*argv, "/columns", 8) == 0)
1034+
else if (extract_argument_value(*argv, {"-columns", "/columns"}, arg_value))
10361035
{
1037-
string cmd = string(*argv);
1038-
size_t found = cmd.find('=',2);
1039-
if (found != string::npos) {
1040-
no_columns = atoi(cmd.substr(found+1).c_str());
1041-
if (no_columns == 0)
1042-
no_columns = DEFAULT_DISPLAY_COLUMNS;
1043-
if (no_columns > m->getNumSockets())
1044-
no_columns = m->getNumSockets();
1036+
if(arg_value.empty()) {
1037+
continue;
10451038
}
1039+
no_columns = stoi(arg_value);
1040+
if (no_columns == 0)
1041+
no_columns = DEFAULT_DISPLAY_COLUMNS;
1042+
if (no_columns > m->getNumSockets())
1043+
no_columns = m->getNumSockets();
10461044
continue;
10471045
}
1048-
else if (strncmp(*argv, "-rank", 5) == 0 ||
1049-
strncmp(*argv, "/rank", 5) == 0)
1046+
else if (extract_argument_value(*argv, {"-rank", "/rank"}, arg_value))
10501047
{
1051-
string cmd = string(*argv);
1052-
size_t found = cmd.find('=',2);
1053-
if (found != string::npos) {
1054-
int rank = atoi(cmd.substr(found+1).c_str());
1055-
if (rankA >= 0 && rankB >= 0)
1056-
{
1057-
cerr << "At most two DIMM ranks can be monitored \n";
1048+
if(arg_value.empty()) {
1049+
continue;
1050+
}
1051+
int rank = stoi(arg_value);
1052+
if (rankA >= 0 && rankB >= 0)
1053+
{
1054+
cerr << "At most two DIMM ranks can be monitored \n";
1055+
exit(EXIT_FAILURE);
1056+
} else {
1057+
if(rank > 7) {
1058+
cerr << "Invalid rank number " << rank << "\n";
10581059
exit(EXIT_FAILURE);
1059-
} else {
1060-
if(rank > 7) {
1061-
cerr << "Invalid rank number " << rank << "\n";
1062-
exit(EXIT_FAILURE);
1063-
}
1064-
if(rankA < 0) rankA = rank;
1065-
else if(rankB < 0) rankB = rank;
1066-
metrics = PartialWrites;
10671060
}
1061+
if(rankA < 0) rankA = rank;
1062+
else if(rankB < 0) rankB = rank;
1063+
metrics = PartialWrites;
10681064
}
10691065
continue;
10701066
}
1071-
else if (strncmp(*argv, "--nochannel", 11) == 0 ||
1072-
strncmp(*argv, "-nc", 3) == 0 ||
1073-
strncmp(*argv, "/nc", 3) == 0)
1067+
else if (check_argument_equals(*argv, {"--nochannel", "/nc", "-nc"}))
10741068
{
10751069
show_channel_output = false;
10761070
continue;
10771071
}
1078-
else if (strncmp(*argv, "-pmm", 4) == 0 ||
1079-
strncmp(*argv, "/pmm", 4) == 0 ||
1080-
strncmp(*argv, "-pmem", 5) == 0 ||
1081-
strncmp(*argv, "/pmem", 5) == 0 )
1072+
else if (check_argument_equals(*argv, {"-pmm", "/pmm", "-pmem", "/pmem"}))
10821073
{
10831074
metrics = Pmem;
10841075
continue;
10851076
}
1086-
else if (strncmp(*argv, "-all", 4) == 0 ||
1087-
strncmp(*argv, "/all", 4) == 0)
1077+
else if (check_argument_equals(*argv, {"-all", "/all"}))
10881078
{
10891079
skipInactiveChannels = false;
10901080
continue;
10911081
}
1092-
else if (strncmp(*argv, "-mixed", 6) == 0 ||
1093-
strncmp(*argv, "/mixed", 6) == 0)
1082+
else if (check_argument_equals(*argv, {"-mixed", "/mixed"}))
10941083
{
10951084
metrics = PmemMixedMode;
10961085
continue;
10971086
}
1098-
else if (strncmp(*argv, "-mm", 3) == 0 ||
1099-
strncmp(*argv, "/mm", 3) == 0)
1087+
else if (check_argument_equals(*argv, {"-mm", "/mm"}))
11001088
{
11011089
metrics = PmemMemoryMode;
11021090
show_channel_output = false;
11031091
continue;
11041092
}
1105-
else if (strncmp(*argv, "-partial", 8) == 0 ||
1106-
strncmp(*argv, "/partial", 8) == 0)
1093+
else if (check_argument_equals(*argv, {"-partial", "/partial"}))
11071094
{
11081095
metrics = PartialWrites;
11091096
continue;
11101097
}
1111-
else if (strncmp(*argv, "-u", 2) == 0 ||
1112-
strncmp(*argv, "/u", 2) == 0)
1098+
else if (check_argument_equals(*argv, {"-u", "/u"}))
11131099
{
11141100
print_update = true;
11151101
continue;
11161102
}
11171103
#ifdef _MSC_VER
1118-
else if (strncmp(*argv, "--uninstallDriver", 17) == 0)
1104+
else if (check_argument_equals(*argv, {"--uninstallDriver"}))
11191105
{
11201106
Driver tmpDrvObject;
11211107
tmpDrvObject.uninstall();
11221108
cerr << "msr.sys driver has been uninstalled. You might need to reboot the system to make this effective.\n";
11231109
exit(EXIT_SUCCESS);
11241110
}
1125-
else if (strncmp(*argv, "--installDriver", 15) == 0)
1111+
else if (check_argument_equals(*argv, {"--installDriver"}))
11261112
{
11271113
Driver tmpDrvObject = Driver(Driver::msrLocalPath());
11281114
if (!tmpDrvObject.start())
@@ -1134,7 +1120,7 @@ int main(int argc, char * argv[])
11341120
exit(EXIT_SUCCESS);
11351121
}
11361122
#endif
1137-
else if (strncmp(*argv, "--", 2) == 0)
1123+
else if (check_argument_equals(*argv, {"--"}))
11381124
{
11391125
argv++;
11401126
sysCmd = *argv;
@@ -1143,18 +1129,7 @@ int main(int argc, char * argv[])
11431129
}
11441130
else
11451131
{
1146-
// any other options positional that is a floating point number is treated as <delay>,
1147-
// while the other options are ignored with a warning issues to stderr
1148-
double delay_input = 0.0;
1149-
istringstream is_str_stream(*argv);
1150-
is_str_stream >> noskipws >> delay_input;
1151-
if(is_str_stream.eof() && !is_str_stream.fail()) {
1152-
delay = delay_input;
1153-
} else {
1154-
cerr << "WARNING: unknown command-line option: \"" << *argv << "\". Ignoring it.\n";
1155-
print_help(program);
1156-
exit(EXIT_FAILURE);
1157-
}
1132+
delay = parse_delay(*argv, program, (print_usage_func)print_help);
11581133
continue;
11591134
}
11601135
} while (argc > 1); // end of command line parsing loop

0 commit comments

Comments
 (0)