Page MenuHomePhabricator

[wip] Move primary globals to globals.h
AbandonedPublic

Authored by schancel on Mon, Nov 26, 19:16.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Test Plan
make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
arcpatch-D2135 (branched from master)
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 4148
Build 6367: Bitcoin ABC Teamcity Staging
Build 6366: arc lint + arc unit

Event Timeline

schancel created this revision.Mon, Nov 26, 19:16
Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Nov 26, 19:16
deadalnix requested changes to this revision.Tue, Nov 27, 13:50
deadalnix added a subscriber: deadalnix.

I would advise strongly against moving these globals around unless you can ensure that initialization will be done in proper order. Initialization across modules is linker dependent, unless you want to specify it using a linker script so they'd better not depend on each other, which I wouldn't bet on.

src/Makefile.am
427 ↗(On Diff #6105)

This being included in 3 libraries indicate that it's likely none of these 3 places is actually correct.

src/logging.cpp
27 ↗(On Diff #6105)

Now you just lost any guarantees on initialization order.

src/qt/bitcoingui.cpp
1157 ↗(On Diff #6105)

The semantic isn't the same anymore. In addition it doesn't look like any of the changes int hat file is related to the diff.

src/qt/coincontroldialog.cpp
18 ↗(On Diff #6105)

Why twice ?

This revision now requires changes to proceed.Tue, Nov 27, 13:50
schancel updated this revision to Diff 6137.Tue, Nov 27, 18:21

Minor cleanup. Still doenst' link

schancel marked an inline comment as done.Tue, Nov 27, 18:23
schancel added inline comments.
src/Makefile.am
427 ↗(On Diff #6105)

Indeed. I'm having trouble with getting it to link. Not sure why: https://reviews.bitcoinabc.org/P21

schancel planned changes to this revision.Wed, Nov 28, 16:45
schancel abandoned this revision.Fri, Nov 30, 21:55