net: remove more CConnman globals #11387

pull theuni wants to merge 4 commits into bitcoin:master from theuni:more-connman-params changing 7 files +70 −67

theuni commented at 11:16 PM on September 22, 2017: member

This continues some of the great work that @benma has been doing.

More work towards a CConnman with no globals, so that we can test instances against themselves.

tor: pass in Tor control port rather than using globals

Also, rather than adding to the existing unused parameters
(threadGroup, scheduler), remove them and their header dependencies.

81aebaf306

net: pass listen port into MapPort/Discover rather than using globals.
This also allows the port-less AddLocal overload to be removed.
0258734054

net: pass listen port into CConnman and remove global GetListenPort

The global test is removed because it's no longer relevant. Overriding via the
-port config option is tested extensively via python tests.

6f2372d8ab

net: pass in default outgoing port 10c496c156
theuni commented at 11:53 PM on September 22, 2017: member

Grr, serves me right for building without Qt. Will fix.
fanquake added the label P2P on Sep 23, 2017
theuni commented at 10:20 PM on September 25, 2017: member

@jonasschnelli Any suggestion where to stick this? The qt build fails because the global accessor has been removed. I'd hate to add the global back just for qt.

Is it possible to add a QSettings value from gArgs without it ending up stored in the config file?
jonasschnelli commented at 9:53 PM on September 26, 2017: contributor

@theuni: I don't know whats the simples change. You could pass it into a GUI constant holder singleton (or pseudo global). Could be held in guiutil.cpp/h. Or just pass it around (to bitcoingui.cpp, etc., but I guess that will lead to a lot of code)
benma commented at 8:47 AM on September 27, 2017: none

This seems to be the only option that changes operations on the fly. We could just not do that (setRestartRequired(true)).

Otherwise, a clean option might be to register a callback in qt, called when non-restart settings are changed. Could hook up the dataChanged signal to a slot somewhere closer in qt to where the callback can be passed.
jonasschnelli commented at 2:55 AM on September 28, 2017: contributor

Concept ACK
fanquake added this to the "In progress" column in a project

MarcoFalke added the label Needs rebase on Jun 6, 2018
DrahtBot commented at 11:20 PM on November 8, 2018: member

There hasn't been much activity lately and the patch still needs rebase, so I am closing this for now. Please let me know when you want to continue working on this, so the pull request can be re-opened.
DrahtBot added the label Up for grabs on Nov 8, 2018
DrahtBot closed this on Nov 8, 2018
MarcoFalke commented at 7:36 PM on March 5, 2019: member

Picked up in "net: remove more CConnman globals (theuni) #14856"
MarcoFalke removed the label Up for grabs on Mar 5, 2019
laanwj removed the label Needs rebase on Oct 24, 2019
MarcoFalke locked this on Dec 16, 2021

Contributors

Labels