Remove double-dash parameters from mapArgs #4193

pull kostaz wants to merge 1 commits into bitcoin:master from kostaz:rm_duplicate_argv changing 1 files +8 −12

kostaz commented at 12:18 PM on May 17, 2014: contributor

If command line parameters contain '--gen' (for example), then 'mapArgs' map will contain both '--gen' and '-gen' after BOOST_FOREACH. As it is redundant, remove '--gen' from the 'mapArgs' map.
laanwj commented at 1:40 PM on May 17, 2014: member

I wonder how this interacts with parameters that can be specified multiple times like --rpcbind/--bind. Does this remove all the occurences?
kostaz commented at 2:51 PM on May 17, 2014: contributor

As the code is built, for '--bind' (for example) parameter mapArgs contains two entries '--bind' and '-bind'. The fix removes the '--bind' entry.
laanwj commented at 4:54 PM on May 17, 2014: member

But what if the user already provides -bind= two times? or, both a -bind and --bind?
laanwj commented at 1:47 PM on June 3, 2014: member

@kostaz I'd like to suggest an alternative implementation as I don't like the current loop which modifies mapArgs while iterating over it.

You could detect and remove the second '-' already before inserting into mapArgs and mapMultiArgs, here: https://github.com/bitcoin/bitcoin/blob/master/src/util.cpp#L482
kostaz commented at 4:42 AM on June 4, 2014: contributor

Updated. Double-dash is removed before BOOST_FOREACH. Please review.
laanwj commented at 6:20 AM on June 4, 2014: member

Code changes look good to me. BTW I think #4281 is needed before this, otherwise --help will stop working.

Remove double-dash parameters from mapArgs

Should be merged after pull request #4281
("Add `-version` option to get just the version #4281"),
because is changed "--help" to "-help".

Checked that grep of 'mapArgs.count("--' returned only
three places that are fixed by pull request #4281.

71aaff393f

kostaz commented at 8:30 AM on June 4, 2014: contributor

Totally right.

Should be merged after pull request #4281 ("Add -version option to get just the version #4281"), because is changed "--help" to "-help".

Checked that grep of 'mapArgs.count("--' returned only three places that are fixed by pull request #4281.
kostaz commented at 8:33 AM on June 4, 2014: contributor

It would be much better to use 'boost::program_options' for all parameter stuff, like here: http://www.boost.org/doc/libs/1_55_0/doc/html/program_options/tutorial.html

Are there any plans/objections to this massive change in parameter handling?
BitcoinPullTester commented at 8:57 AM on June 4, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/71aaff393f06378150612fc618b6c3f84c1d2066 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
laanwj merged this on Jun 12, 2014
laanwj closed this on Jun 12, 2014
laanwj referenced this in commit 8047fb04ed on Jun 12, 2014
DrahtBot locked this on Sep 8, 2021

Contributors

kostaz

laanwj

BitcoinPullTester