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

check input args #264

Merged
merged 8 commits into from
Nov 29, 2019
Merged

check input args #264

merged 8 commits into from
Nov 29, 2019

Conversation

86667
Copy link

@86667 86667 commented Nov 27, 2019

src/util.h Show resolved Hide resolved
Copy link

@lawlawlaw lawlawlaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be fine once you've updated util_tests.cpp.

src/util.cpp Outdated Show resolved Hide resolved
@@ -102,7 +102,7 @@ BOOST_AUTO_TEST_CASE(util_DateTimeStrFormat)

BOOST_AUTO_TEST_CASE(util_ParseParameters)
Copy link

@lawlawlaw lawlawlaw Nov 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the old test case, arguments were either ignored, or they ended up both MapArgs and MapMultiArgs.

-ignored is ignored anyway because it is the first argument (program name) so it should not be in the valid arguments list.

The new behaviour we want is for invalid arguments to result in an error so this test needs to change.

Add more test cases parsing invalid parameters and include some try/catch statements and assert that the correct error was thrown.

Are -par and -pid really valid program arguments or are the included for the sake of the test?

I see that "f" and everything after it is ignored. I can't think of why that would be the best outcome from missing out a "-" in the arguments list. Consider modifying the code to throw an error in this case as well.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-par sets the number of script verification threads and -pid sets the location of bitcoin.pid the file.

@nkostoulas nkostoulas merged commit 174f1f4 into master Nov 29, 2019
@nkostoulas nkostoulas deleted the input_arg_check branch November 29, 2019 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants