Util: Remove redundant calls to argsGlobal.IsArgSet() #10118

pull jtimon wants to merge 2 commits into bitcoin:master from jtimon:0.14-args-class2 changing 5 files +77 −98

jtimon commented at 11:20 PM on March 29, 2017: contributor
Includes:
- Introduce an ArgsManager class encapsulating cs_args, mapArgs and mapMultiArgs #9494
I'm happy squashing the indentation commit, leaving it separated or leaving it out of the PR.
fanquake added the label Refactoring on Mar 30, 2017
jtimon force-pushed on Apr 4, 2017
jtimon commented at 3:27 PM on April 4, 2017: contributor

Needed rebase.
jtimon force-pushed on Apr 5, 2017
jtimon force-pushed on Apr 6, 2017
jtimon force-pushed on May 19, 2017
jtimon commented at 3:31 AM on May 19, 2017: contributor

Needed rebase after #9494 was merged. The indentation is improved for the first commit but the second commit making the indentation complete was not rebased and was removed instead. If it is wanted I am happy to re-write it. Also, before this wasn't passing the python tests because it needed the change in ArgsManager::GetArgs().

in src/httprpc.cpp:97 in 6fe4c68524 outdated

  92 | @@ -93,10 +93,8 @@ static bool multiUserAuthorized(std::string strUserPass)
  93 |      std::string strUser = strUserPass.substr(0, strUserPass.find(":"));
  94 |      std::string strPass = strUserPass.substr(strUserPass.find(":") + 1);
  95 |  
  96 | -    if (gArgs.IsArgSet("-rpcauth")) {
  97 | -        //Search for multi-user login/pass "rpcauth" from config
  98 | -        BOOST_FOREACH(std::string strRPCAuth, gArgs.GetArgs("-rpcauth"))
  99 | -        {
 100 | +    //Search for multi-user login/pass "rpcauth" from config
 101 | +    BOOST_FOREACH(std::string strRPCAuth, gArgs.GetArgs("-rpcauth")) {

TheBlueMatt commented at 8:13 PM on May 29, 2017:

We generally prefer to remove BOOST while changing lines, no (also, I believe could constify the strRPCAuth, while you're at it)?

jtimon commented at 9:54 PM on May 29, 2017:

Yeah, I was leaving it for #10193, but happy to do it here in the touched lines. Specially since I'm not able to fix my problem with the reverse loop there :(

TheBlueMatt commented at 8:25 PM on May 29, 2017: member

Note you can also remove the IsArgSet("-addnode") at the top of ThreadOpenAddedConnections.

Also, would highly prefer you not leave indentation broken everywhere - its easy enough to review with whitespace diff ignored.
jtimon commented at 10:19 PM on May 29, 2017: contributor

Fixed nits, squash pending.
jtimon force-pushed on May 29, 2017
jtimon force-pushed on Jun 1, 2017
jtimon commented at 6:48 PM on June 1, 2017: contributor

Needed rebase, squashed fixes to @TheBlueMatt 's nits.
jtimon force-pushed on Jun 14, 2017
jtimon commented at 4:01 AM on June 14, 2017: contributor

Needed rebase after #10502 was merged. Now it's slightly easier to review in the sense that #10118 (review) which was fixed as nit in the PR is now irrelevant.

Extra motivation for review: "C'mon, +16 -37, must be good".

in src/util.cpp:425 in b527f8d3b1 outdated

 419 | @@ -420,7 +420,9 @@ void ArgsManager::ParseParameters(int argc, const char* const argv[])
 420 |  std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg)
 421 |  {
 422 |      LOCK(cs_args);
 423 | -    return mapMultiArgs.at(strArg);
 424 | +    if (IsArgSet(strArg))
 425 | +        return mapMultiArgs.at(strArg);
 426 | +    return std::vector<std::string>();

ryanofsky commented at 7:06 PM on June 14, 2017:

Could return {};

ryanofsky commented at 7:09 PM on June 14, 2017: member

utACK b527f8d3b10ee2232a921faf934e690af0e14640
jtimon force-pushed on Jun 17, 2017
jtimon commented at 6:25 PM on June 17, 2017: contributor

Fixed @ryanofsky 's nit.
ryanofsky commented at 6:24 PM on June 21, 2017: member

utACK a0c00786d488a1022867e7461ff071dd78d2361e. Same as previous, just has the suggested change.
MarcoFalke commented at 9:02 AM on June 22, 2017: member

utACK a0c00786d488a1022867e7461ff071dd78d2361e
laanwj commented at 9:03 AM on June 25, 2017: member

Concept ACK
jtimon commented at 8:57 PM on June 25, 2017: contributor

If it helps I can do the proper indentation in a commit afterwards, I had it previously but I ditched on rebase out of laziness at some point. Trivial to put it back.
laanwj commented at 10:59 AM on June 26, 2017: member

I'd prefer fixing up the identation in this same PR, a commit afterwards is fine, but I'd prefer we avoid a "fix up the identation after #10118" PR. utACK otherwise.

Util: Remove redundant calls to gArgs.IsArgSet()

Return empty std::vector<std::string> with ArgsManager::GetArgs if
nothing is set for that string

506b700dcb

jtimon force-pushed on Jun 27, 2017
jtimon commented at 1:12 AM on June 27, 2017: contributor

Needed rebase, added commit for indentation.
Indentation after 'Remove redundant calls to gArgs.IsArgSet()' ed866ab923
jtimon force-pushed on Jun 27, 2017
laanwj merged this on Jun 27, 2017
laanwj closed this on Jun 27, 2017
laanwj referenced this in commit ac52492cd2 on Jun 27, 2017
PastaPastaPasta referenced this in commit 7159b6a93f on Jul 6, 2019
PastaPastaPasta referenced this in commit 741747446f on Jul 6, 2019
PastaPastaPasta referenced this in commit bb45a3daae on Jul 6, 2019
PastaPastaPasta referenced this in commit 7df59510b8 on Jul 8, 2019
PastaPastaPasta referenced this in commit 4f33a22771 on Jul 9, 2019
PastaPastaPasta referenced this in commit e19fc0b9b2 on Jul 11, 2019
barrystyle referenced this in commit cb41cd815e on Jan 22, 2020
furszy referenced this in commit de5b240881 on Oct 28, 2020
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#9494 Introduce an ArgsManager class encapsulating cs_args, mapArgs and mapMultiArgs