refactor: Stop using gArgs global in system.cpp #27170
Conversation
New function was introduced by willcl-ark <will@256k1.dev> in commit 56e370f from bitcoin#27073 and removes some duplicate code.
Most of the code in util/system.cpp that was hardcoded to use the global ArgsManager instance `gArgs` has been changed to work with explicit ArgsManager instances (for example in bitcoin#20092). But a few hardcoded references to `gArgs` remain. This commit removes the last ones so these functions aren't reading or writing global state.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
Concept ACK |
|
Concept ACK |
|
Concept ACK. |
There was a problem hiding this comment.
tACK 9a9d5da
Tidy changes to remove the last references to global gArgs from util/system.cpp (besides the global declaration).
| @@ -1068,8 +1068,8 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) | |||
| } | |||
|
|
|||
| // If datadir is changed in .conf file: | |||
| gArgs.ClearPathCache(); | |||
There was a problem hiding this comment.
nit: Looks like this shouldn't have been called on gArgs regardless of this PR, perhaps useful to split out in separate commit? In the 3 callsites of ArgsManager::ReadConfigFiles I see, it is only called on gArgs anyway, so I don't see any actual behaviour change/potential bug because of it.
|
ACK 9a9d5da |
Most of the code in
util/system.cppthat was hardcoded to use the globalArgsManagerinstancegArgshas been changed to stop using it (for example in #20092). But a few hardcoded references togArgsremain. This commit removes the last ones so these functions aren't reading or writing global state.Noticed these
gArgsreferences while reviewing #27073