Page MenuHomePhabricator
Differential D4659

Replace integer literals in dnshandle() with an enum class
ClosedPublic
Actions

Authored by nakihito on Dec 6 2019, 18:36.

Details

Reviewers
deadalnix
Fabien
jasonbcox
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rSTAGING767621608503: Replace integer literals in dnshandle() with an enum class
rABC767621608503: Replace integer literals in dnshandle() with an enum class
Summary

The error values used in seeder/dns.cpp are defined in RFC1035 under
RCODE. The integer literals currently used as error values can be
replaced with an enum class with accurate names in line with RFC1035.

Test Plan
make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
DNSErrorConsts
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 8536
Build 15066: Default Diff Build & Tests
Build 15065: arc lint + arc unit

Event Timeline

nakihito created this revision.Dec 6 2019, 18:36
Owners added a reviewer: Restricted Owners Package.Dec 6 2019, 18:36
Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 6 2019, 18:36
nakihito planned changes to this revision.Dec 6 2019, 18:36
teamcity added a comment.Dec 6 2019, 18:36

Build Bitcoin-ABC / Diffs / Diff Testing started.

teamcity added a comment.Dec 6 2019, 18:50

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8443: Diff 14677.Dec 6 2019, 18:50
nakihito requested review of this revision.Dec 6 2019, 19:00
nakihito updated this revision to Diff 14685.Dec 6 2019, 23:32

Changed constant ints to an enum.

teamcity added a comment.Dec 6 2019, 23:32

Build Bitcoin-ABC / Diffs / Diff Testing started.

nakihito edited the summary of this revision. (Show Details)Dec 6 2019, 23:32
teamcity added a comment.Dec 6 2019, 23:43

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8448: Diff 14685.Dec 6 2019, 23:43
nakihito retitled this revision from Replace integer literals in dnshandle() with constants to Replace integer literals in dnshandle() with an enum.Dec 6 2019, 23:54
deadalnix requested changes to this revision.Dec 8 2019, 01:57
deadalnix added inline comments.
src/seeder/dns.h
18 ↗(On Diff #14685)

Why is this part of the public API, why does this have smurf naming and why is no_error an error?

This revision now requires changes to proceed.Dec 8 2019, 01:57
nakihito updated this revision to Diff 14705.Dec 9 2019, 19:46

Renamed variable error -> responseCode, fixed smurf naming, and moved enum to dns.cpp.

nakihito planned changes to this revision.Dec 9 2019, 19:46
teamcity added a comment.Dec 9 2019, 19:46

Build Bitcoin-ABC / Diffs / Diff Testing started.

nakihito requested review of this revision.Dec 9 2019, 19:47
teamcity added a comment.Dec 9 2019, 19:58

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8458: Diff 14705.Dec 9 2019, 19:58
deadalnix requested changes to this revision.Dec 10 2019, 03:37
deadalnix added inline comments.
src/seeder/dns.cpp
56 ↗(On Diff #14705)

You can make that an enum class so it is strongly typed and names are scoped.

Also DNSResponseCode for types.

This revision now requires changes to proceed.Dec 10 2019, 03:37
nakihito updated this revision to Diff 14728.Dec 10 2019, 19:05

Changed enum to enum class.

nakihito planned changes to this revision.Dec 10 2019, 19:05
teamcity added a comment.Dec 10 2019, 19:05

Build Bitcoin-ABC / Diffs / Diff Testing started.

nakihito requested review of this revision.Dec 10 2019, 19:07
nakihito retitled this revision from Replace integer literals in dnshandle() with an enum to Replace integer literals in dnshandle() with an enum class.
nakihito edited the summary of this revision. (Show Details)
teamcity added a comment.Dec 10 2019, 19:16

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8478: Diff 14728.Dec 10 2019, 19:16
deadalnix requested changes to this revision.Dec 11 2019, 02:29
deadalnix added inline comments.
src/seeder/dns.cpp
57 ↗(On Diff #14728)

OK or something. NO_ERROR is kind of a bizarre response code.

572 ↗(On Diff #14728)

Just use a constructor and not a cast.

This revision now requires changes to proceed.Dec 11 2019, 02:29
nakihito updated this revision to Diff 14782.Dec 11 2019, 21:25

static_cast<uint8_t>() -> uint8_t()

teamcity added a comment.Dec 11 2019, 21:25

Build Bitcoin-ABC / Diffs / Diff Testing started.

teamcity added a comment.Dec 11 2019, 21:36

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8510: Diff 14782.Dec 11 2019, 21:36
nakihito updated this revision to Diff 14819.Dec 13 2019, 00:08

Disucssed with Jason offline, NO_ERROR -> OK.

teamcity added a comment.Dec 13 2019, 00:08

Build Bitcoin-ABC / Diffs / Diff Testing started.

teamcity added a comment.Dec 13 2019, 00:28

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8536: Diff 14819.Dec 13 2019, 00:28
deadalnix accepted this revision.Dec 17 2019, 23:48

likestamp

This revision is now accepted and ready to land.Dec 17 2019, 23:48
Closed by commit rABC767621608503: Replace integer literals in dnshandle() with an enum class (authored by nakihito). · Explain WhyDec 18 2019, 00:17
This revision was automatically updated to reflect the committed changes.
nakihito added a commit: rABC767621608503: Replace integer literals in dnshandle() with an enum class.
nakihito added a commit: rSTAGING767621608503: Replace integer literals in dnshandle() with an enum class.Apr 7 2020, 22:07

Revision Contents
Changeset List

PathSize
src/
seeder/
33 lines

Diff 14819

View Options

src/seeder/dns.cpp

Loading...