Page MenuHomePhabricator

Numerous improvement to the seeder's code
ClosedPublic

Authored by deadalnix on Sep 26 2017, 19:15.

Details

Summary
  • unsigned char => uint8_t
  • C++ style imprts
  • project import before library imports
  • Relocate some comments for better layout
  • Reorder inline and static qualifiers
  • Remove blanket namespace imports
  • Upgrade some of the loops for C++11 style loops
Test Plan

Compile and run the seeder.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

deadalnix created this revision.Sep 26 2017, 19:15
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 26 2017, 19:15
deadalnix updated this revision to Diff 1417.Sep 26 2017, 19:24

Also take care of standalone headers

deadalnix updated this revision to Diff 1422.Sep 26 2017, 22:30

Also do util.cpp

deadalnix updated this revision to Diff 1426.Sep 27 2017, 10:19

Do db.cpp

sickpig added inline comments.Sep 27 2017, 15:52
src/seeder/bitcoin.cpp
26 ↗(On Diff #1426)

don't we usually got to a new line also if the then branch is a one-liner?

src/seeder/main.cpp
439 ↗(On Diff #1426)

don't recall BU having a cash seeder for the testnet

src/seeder/netbase.cpp
835 ↗(On Diff #1426)

why not uint something?

src/seeder/serialize.h
135 ↗(On Diff #1426)

unsigned int?
(same question for all the belove)

sickpig requested changes to this revision.Sep 27 2017, 15:52
This revision now requires changes to proceed.Sep 27 2017, 15:52
deadalnix requested review of this revision.Sep 27 2017, 16:07
deadalnix edited edge metadata.
deadalnix added inline comments.
src/seeder/bitcoin.cpp
26 ↗(On Diff #1426)

We do whatever the formatter does.

src/seeder/main.cpp
439 ↗(On Diff #1426)

It's in ABC's source code. Someone must have given us this address.

src/seeder/netbase.cpp
835 ↗(On Diff #1426)

I don't know, it was an int and it is still an int :)

src/seeder/serialize.h
135 ↗(On Diff #1426)

We aren't sending message more than 4Gb anyway so it doesn't matter. Plus the whole serialization code is trashed in another diff.

sickpig added inline comments.Sep 28 2017, 09:14
src/seeder/bitcoin.cpp
26 ↗(On Diff #1426)

fair point.

hope that clang-format does not develop an evil form of AI otherwise... 😄

src/seeder/main.cpp
439 ↗(On Diff #1426)

copying something wrong doesn't making correct magically

just checked there's no testnet-seed.bitcoinunlimited.info.

if it has being taken from other part of the ABC's that means that there's another place in the code to fix.

src/seeder/netbase.cpp
835 ↗(On Diff #1426)

fair

src/seeder/serialize.h
135 ↗(On Diff #1426)

fair

deadalnix added inline comments.Sep 28 2017, 16:42
src/seeder/main.cpp
439 ↗(On Diff #1426)
sickpig accepted this revision.Sep 28 2017, 19:46
This revision is now accepted and ready to land.Sep 28 2017, 19:46
This revision was automatically updated to reflect the committed changes.