Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

argv is missing nullptr at the end in TestParallelHarness #1199

Closed
cz4rs opened this issue Dec 29, 2020 · 1 comment · Fixed by #1200
Closed

argv is missing nullptr at the end in TestParallelHarness #1199

cz4rs opened this issue Dec 29, 2020 · 1 comment · Fixed by #1200
Assignees

Comments

@cz4rs
Copy link
Contributor

cz4rs commented Dec 29, 2020

Describe the bug
For strict standards compliance (i.e. if the parser in cli11 relies on it now or in the future), nullptr should be pushed on the end of a vector storing argv.

TestParallelHarness uses additional_args_ to store arguments, which lacks nullptr at argv[argc].

std::pair<int, char**>
injectAdditionalArgs(int old_argc, char** old_argv) {
std::copy(
old_argv, old_argv + old_argc, std::back_inserter(additional_args_)
);
addAdditionalArgs();
int custom_argc = additional_args_.size();
char** custom_argv = additional_args_.data();
return std::make_pair(custom_argc, custom_argv);
}
/**
* \internal \brief Add additional arguments used during initialization of vt
* components
*
* To add additional arguments override this function in your class and add
* needed arguments to `additional_args_` vector.
*
* Example:
* struct TestParallelHarnessWithStatsDumping : TestParallelHarnessParam<int> {
* virtual void addAdditionalArgs() override {
* static char vt_lb_stats[]{"--vt_lb_stats"};
* static char vt_lb_stats_dir[]{"--vt_lb_stats_dir=test_stats_dir"};
* static char vt_lb_stats_file[]{"--vt_lb_stats_file=test_stats_outfile"};
*
* addArgs(vt_lb_stats, vt_lb_stats_dir, vt_lb_stats_file);
* }
* };
*/
virtual void addAdditionalArgs() {}

Expected behavior
Add nullptr as required by the standard. See 1ce0310 for reference.

@cz4rs
Copy link
Contributor Author

cz4rs commented Dec 29, 2020

note: TestHarness seems to be missing it as well

virtual void SetUp() {
argc_ = orig_args_.size();
argv_ = new char*[argc_];
for (int i = 0; i < argc_; i++) {
auto const len = orig_args_[i].size();
argv_[i] = new char[len + 1];
argv_[i][len] = 0;
orig_args_[i].copy(argv_[i], len);
}
}
virtual void TearDown() {
for (int i = 0; i < argc_; i++) {
delete [] argv_[i];
}
delete [] argv_;
}
static void store_cmdline_args(int argc, char **argv) {
orig_args_ = std::vector<std::string>(argv, argv + argc);
}

edit: vt::tests::unit::TestHarnessAny<TestBase>::argv_ and vt::tests::unit::TestHarnessAny<TestBase>::argc_ are not used anywhere else and can probably be safely deleted (along with SetUp and TearDown methods)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant